fix(comment-gate): no-finding bot summaries keep approval on hands-off PRs

docs/architecture/adr/0012-comment-gate-exemption-set.md

@@ -56,13 +56,30 @@ We will narrow #1813's "every PR issue comment" scope to every PR issue comment
    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.
+6. **A no-finding review summary** (added after PRs 2456 and 2467 deadlocked
+   under `dev-lead:hands-off`): a Codex or CodeRabbit summary comment whose
+   bot reports in its own structured state that the review found nothing, and
+   whose body carries no finding-bearing section. Codex: exactly one
+   `codex-review-state:v1` JSON with a `completed` code review,
+   `has_any_findings` and `has_unresolved_findings` both `false`, and a security
+   review that never ran or completed the same way. CodeRabbit: exactly one
+   `final_review_risk_label` field reading Merge Risk `⚪ Minimal`, exactly one
+   `final_review_risk_coverage` JSON with kind `reviewed`, and only `passed`
+   pre-merge check badges. The author must be that GitHub App (`Bot`). The match
+   is on the structured state, never on prose, so it holds however often the bot
+   edits the comment (`_MAINTAINER_GATE_NO_FINDING_JQ_DEFS`).
 
 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
 unless the maintainer opts it out. Fail closed holds: a missing
 `authorAssociation` grants neither exemption 3 nor 4, and an unreadable
 snapshot still blocks.
 
+Exemptions 5 and 6 are shared the same way (`_MAINTAINER_GATE_INFO_JQ_DEFS`,
+`_MAINTAINER_GATE_NO_FINDING_JQ_DEFS`), and exemption 6 also by the #2008
+re-open and stale-disposition lists and the bot-comment retry, so nothing
+re-opens, chases or retries a summary the gate clears.
+
 We will keep exemptions 1–4 in one definition (`_MAINTAINER_GATE_SCOPE_JQ_DEFS`)
 used by both the gate and dev-lead's candidate filter
 (`maintainer_gate_open_comment_ids`, called by `resolve_dispositioned_comments`),

docs/pr-review-agent/maintainer-comment-gate.md

@@ -134,6 +134,19 @@ minimized with classifier `RESOLVED`. In practice:
   reaches the branch. The harness first edits the reply so its marker cites the
   rebased successor that landed (#2156), then judges that commit by the same
   rules.
+- **A no-finding Codex or CodeRabbit summary.** Codex and CodeRabbit each keep one
+  summary comment and edit it on every push. Under #2008 every edit needed a fresh
+  dev-lead disposition, so on a `dev-lead:hands-off` PR (no dev-lead) pr-review
+  approved and then dismissed its own approval (PRs 2456 and 2467). A summary whose
+  bot says, in its own structured state, that the review found nothing now needs no
+  disposition, edited or not. Codex: one `codex-review-state:v1` JSON with a
+  `completed` code review whose `has_any_findings` and `has_unresolved_findings` are
+  `false` (the security review never ran, or completed the same way). CodeRabbit:
+  one `final_review_risk_label` reading Merge Risk `⚪ Minimal` (a `🔵 Low` rating
+  comes with follow-ups and still blocks), one `final_review_risk_coverage` JSON with
+  kind `reviewed`, and only `passed` pre-merge check badges. The author must be the
+  GitHub App (`Bot`), and a finding-bearing section still blocks. Any other state —
+  failed, in progress, findings, unparseable, a second state block — still blocks.
 - **A maintainer clears it without dev-lead (#1910/#1918).** When dev-lead is
   suppressed, rate-limited, cancelled, or never dispatched, a maintainer can run
   [`scripts/maintainer-resolve-comment.sh`](../../scripts/maintainer-resolve-comment.sh)

scripts/dev-lead-fix-reviews.sh

@@ -1999,8 +1999,12 @@ resolve_dispositioned_comments() {
       unminimize_failed=1
     fi
   done <<< "$reopen_ids"
-  reverify_ids=$(printf '%s' "$all_comments" | jq -r --arg reopen "$reopen_ids" '
-      ($reopen | split("\n")) as $skip
+  # A no-finding Codex or CodeRabbit summary is cleared by the gate whatever its
+  # edit history, so it is never re-verified (and so never unminimized).
+  local no_finding_ids
+  no_finding_ids=$(maintainer_gate_no_finding_ids "$all_comments" || true)
+  reverify_ids=$(printf '%s' "$all_comments" | jq -r --arg reopen "$reopen_ids" --arg nf "$no_finding_ids" '
+      (($reopen + "\n" + $nf) | split("\n")) as $skip
       | .[] | objects
       | select((.author?.__typename // "") == "Bot")
       | select(((.isMinimized // false) == true)

scripts/lib/bot-comment-retry.sh

@@ -19,8 +19,8 @@
 #   For each candidate comment — authored by a trusted registered reviewer bot,
 #   not agent-marked, not minimized RESOLVED (unless it was edited after its
 #   newest covering disposition, which the gate re-blocks: #2008, #2166), not
-#   cleared by its source's info_status_pattern (the same filters the gate
-#   applies) — whether to dispatch
+#   cleared by its source's info_status_pattern, not a no-finding Codex or
+#   CodeRabbit summary (the same filters the gate applies) — whether to dispatch
 #   a fix-bot-comment retry now. A comment's VERSION is lastEditedAt // createdAt;
 #   all coverage checks are against the current version, so an edit re-opens it.
 #     dispositioned            a `dev-lead:comment-disposition id=<id>` reply exists
@@ -180,6 +180,8 @@ _bcr_evaluate() {
   [[ "$skew" =~ ^[0-9]+$ ]] || skew=2
   local lost_backoff="${BOT_COMMENT_RETRY_LOST_BACKOFF_SEC:-$min_age}"
   [[ "$lost_backoff" =~ ^[0-9]+$ ]] || lost_backoff="$min_age"
+  local finding_re
+  finding_re="$(_maintainer_gate_finding_re)"
 
   printf '%s' "$comments" | jq -ce \
     --arg mode "$mode" \
@@ -197,7 +199,8 @@ _bcr_evaluate() {
     --argjson max_attempts "$max_attempts" \
     --argjson max_total "$max_total" \
     --argjson skew "$skew" \
-    --arg botuser "${BOT_USER:-donpetry-bot}" "$_MAINTAINER_GATE_DISP_JQ_DEFS"'
+    --arg findre "$finding_re" \
+    --arg botuser "${BOT_USER:-donpetry-bot}" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
     def bare: if endswith("[bot]") then .[0:-5] else . end;
     def epoch: (sub("\\.[0-9]+Z$"; "Z") | fromdateiso8601);
     # A marker-supplied timestamp: its epoch, or null when absent/unparseable.
@@ -254,6 +257,9 @@ _bcr_evaluate() {
         | select((.author?.__typename // "") == "Bot")
         | select(((.body // "") | test($markers)) | not)
         | select(($info[$l] // null) as $p | ($p == null) or (((.body // "") | test($p)) | not))
+        # A no-finding Codex or CodeRabbit summary needs no disposition (the gate
+        # clears it), so it never needs a fix-bot-comment pass either.
+        | select(no_finding_summary | not)
         # A RESOLVED comment is done, unless it was edited after its newest
         # covering disposition (or has none): the gate re-blocks exactly that
         # (#2008), so it needs a fresh pass like an undispositioned one (#2166).

scripts/lib/maintainer-comment-gate.sh

@@ -41,7 +41,9 @@
 #     • from an OWNER/MEMBER/COLLABORATOR only: the explicit opt-out marker
 #       `<!-- maintainer:not-a-finding -->`;
 #     • a registered clean info-status bot comment (#1918), from a GitHub App
-#       author (`author.__typename` "Bot") only.
+#       author (`author.__typename` "Bot") only;
+#     • a Codex or CodeRabbit summary whose own structured state reports no
+#       finding (_MAINTAINER_GATE_NO_FINDING_JQ_DEFS), from a GitHub App author 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,
@@ -148,6 +150,79 @@ readonly _MAINTAINER_GATE_INFO_JQ_DEFS='
            and ($b | test($p)));
 '
 
+# The no-finding summary classifier as jq definitions. Codex and CodeRabbit each
+# keep ONE summary comment per PR and edit it on every push. Under #2008 each edit
+# needs a fresh dev-lead disposition, and on a `dev-lead:hands-off` PR dev-lead
+# posts none, so pr-review approved and then dismissed its own approval (combsmith/
+# .github-private PRs 2456 and 2467). A summary whose bot reports, in its own
+# structured state, that the review found nothing needs no disposition. The match
+# is on that state, never on prose. The caller binds $findre
+# (_maintainer_gate_finding_re).
+#   codex_no_finding     — the body opens with Codex's summary marker and carries
+#                    exactly one `codex-review-state:v1` JSON; that JSON says the
+#                    code review is `completed` with has_any_findings and
+#                    has_unresolved_findings both `false` for a 40-hex commit_sha,
+#                    and a security_review object carrying all four fields that
+#                    either never ran (every field null) or completed the same way
+#                    for that same commit. Any other state (failed, in progress,
+#                    findings, a null, partial or other-commit security review,
+#                    unparseable) is not cleared.
+#   coderabbit_no_finding — the body opens with CodeRabbit's summary marker (after
+#                    its optional `coderabbit-ai-generated:v1` marker) and
+#                    carries exactly one `final_review_risk_label` field reading
+#                    Merge Risk "⚪ Minimal" (a "🔵 Low" rating comes with
+#                    follow-ups, so it is not cleared), exactly one
+#                    `final_review_risk_coverage` JSON with kind "reviewed", and
+#                    pre-merge check badges that are all "passed" (at least one).
+#   no_finding_summary   — ALL of: a GitHub App author (author.__typename "Bot",
+#                    as info_status_clear requires); its bare login is the bot the
+#                    recognizer belongs to; the recognizer matches; and the body
+#                    carries no finding-bearing section (#2008). A missing
+#                    __typename is not "Bot" (fail closed).
+readonly _MAINTAINER_GATE_NO_FINDING_JQ_DEFS='
+  def nf_sha: (type == "string") and test("^[0-9a-f]{40}$");
+  def nf_json($re):
+    (first(capture($re) | .j) // null)
+    | if . == null then null else (try fromjson catch null) end;
+  def codex_no_finding:
+    ((.body // "") | tostring) as $b
+    | ([$b | scan("codex-review-state")] | length) == 1
+      and (($b | nf_json("\\A<!-- codex-pull-request-review-summary -->\\s*<!-- codex-review-state:v1 (?<j>\\{[^\\n]*\\}) -->")) as $s
+        | ($s | type) == "object"
+          and (($s.code_review // null) | type) == "object"
+          and $s.code_review.status == "completed"
+          and $s.code_review.has_any_findings == false
+          and $s.code_review.has_unresolved_findings == false
+          and ($s.code_review.commit_sha | nf_sha)
+          and ($s.code_review.commit_sha as $sha
+               | ($s.security_review // null) as $r
+               | (($r | type) == "object")
+                 and ($r | has("status") and has("has_any_findings")
+                           and has("has_unresolved_findings") and has("commit_sha"))
+                 and (([$r.status, $r.has_any_findings, $r.has_unresolved_findings, $r.commit_sha]
+                       | all(. == null))
+                      or ($r.status == "completed"
+                          and $r.has_any_findings == false
+                          and $r.has_unresolved_findings == false
+                          and $r.commit_sha == $sha))));
+  def coderabbit_no_finding:
+    ((.body // "") | tostring) as $b
+    | ($b | test("\\A(?:<!-- coderabbit-ai-generated:v1 -->\\s*)?<!-- This is an auto-generated comment: summarize by coderabbit\\.ai -->"))
+      and ([$b | scan("final_review_risk_label")] | length) == 1
+      and ($b | test("<!-- final_review_risk_label\\r?\\n\\*\\*Merge Risk:\\*\\* \\*\\*⚪ Minimal\\*\\* · up to `[0-9a-f]{5,40}`\\r?\\n-->"))
+      and ([$b | scan("final_review_risk_coverage")] | length) == 1
+      and ($b | nf_json("<!-- final_review_risk_coverage:(?<j>\\{[^\\n]*\\}) -->")
+           | (type == "object") and .kind == "reviewed"
+             and (.sourceCommitId | nf_sha) and (.coveredCommitId | nf_sha))
+      and ([$b | scan("pre-merge-checks-(?:counter-)?([a-z]+)[-.]") | .[0]] | unique) == ["passed"];
+  def no_finding_summary:
+    ((.author?.__typename // "") == "Bot")
+    and (((.body // "") | tostring | test($findre)) | not)
+    and ((.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $l
+         | ($l == "chatgpt-codex-connector" and codex_no_finding)
+           or ($l == "coderabbitai" and coderabbit_no_finding));
+'
+
 # 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
@@ -420,13 +495,16 @@ maintainer_gate_reopen_candidates() {
   printf '%s' "$comments" | jq -c \
     --arg botuser "$bot_user" \
     --arg findre "$finding_re" \
-    --argjson registered "$registered" "$_MAINTAINER_GATE_DISP_JQ_DEFS"'
+    --argjson registered "$registered" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
     . as $all
     | if type != "array" then error("not an array") else . end
     | [ .[] | objects
         | (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
         | select(((.author?.__typename // "") == "Bot")
                  or ($registered != null and ($registered | index($lbare)) != null))
+        # The gate clears a no-finding summary whatever its edit history, so
+        # there is nothing to re-open.
+        | select(no_finding_summary | not)
         | select(((.isMinimized // false) == true)
                  and (((.minimizedReason // "") | ascii_downcase) == "resolved"))
         | select((.id // "") != "")
@@ -452,17 +530,21 @@ maintainer_gate_reopen_candidates() {
 #   Unreadable input echoes nothing and returns 1, so a caller never dispatches on
 #   a guess. Pure apart from reading the registry.
 maintainer_gate_stale_dispositions() {
-  local comments="${1:-}" bot_user="${2:-donpetry-bot}" registered
+  local comments="${1:-}" bot_user="${2:-donpetry-bot}" registered finding_re
   registered="$(_maintainer_gate_registered_logins_json)"
+  finding_re="$(_maintainer_gate_finding_re)"
   printf '%s' "$comments" | jq -c \
     --arg botuser "$bot_user" \
-    --argjson registered "$registered" "$_MAINTAINER_GATE_DISP_JQ_DEFS"'
+    --arg findre "$finding_re" \
+    --argjson registered "$registered" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
     . as $all
     | if type != "array" then error("not an array") else . end
     | [ .[] | objects
         | (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
         | select(((.author?.__typename // "") == "Bot")
                  or ($registered != null and ($registered | index($lbare)) != null))
+        # A no-finding summary needs no disposition, so a stale one is not chased.
+        | select(no_finding_summary | not)
         # RESOLVED (cleared but stale), or not minimized at all (already re-opened
         # by the harness and still awaiting a fresh disposition). A comment
         # minimized for any other reason (OUTDATED, …) is not ours to chase.
@@ -520,11 +602,11 @@ maintainer_gate_head_committer_date() {
 #   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,__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.
+#   minimizedReason}. Filters out clean registered info-status comments and no-finding
+#   summaries with the same info_status_clear and no_finding_summary definitions
+#   check_maintainer_comments uses, so the gate and candidate filter never disagree.
+#   A node without author.__typename "Bot" is never filtered as either. 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
   info_patterns="$(_maintainer_gate_info_patterns_json)"
@@ -533,18 +615,35 @@ 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$_MAINTAINER_GATE_INFO_JQ_DEFS"'
+    --argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_SCOPE_JQ_DEFS$_MAINTAINER_GATE_INFO_JQ_DEFS$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
       if type != "array" then error("not an array") else . end
       | [ .[] | objects
           | select(in_gate_scope)
           | select(info_status_clear | not)
+          | select(no_finding_summary | not)
           | select(((.isMinimized // false) == true)
                    and (((.minimizedReason // "") | ascii_downcase) == "resolved") | not)
           | .id ]
       | .[]
     ' 2>/dev/null
 }
 
+# maintainer_gate_no_finding_ids <comments_array_json>
+#   Echoes one node id per line for every no-finding Codex or CodeRabbit summary
+#   (no_finding_summary). dev-lead's resolver skips these when it builds its
+#   re-verify list: the gate clears them whatever their edit history, so there is
+#   nothing to verify and nothing to unminimize. Unreadable input echoes nothing
+#   and returns non-zero.
+maintainer_gate_no_finding_ids() {
+  local comments="${1:-}" finding_re
+  finding_re="$(_maintainer_gate_finding_re)"
+  printf '%s' "$comments" | jq -r \
+    --arg findre "$finding_re" "$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
+      if type != "array" then error("not an array") else . end
+      | .[] | objects | select(no_finding_summary) | (.id // empty)
+    ' 2>/dev/null
+}
+
 # check_maintainer_comments <pr_snapshot_json> [bot_user]
 check_maintainer_comments() {
   local json="${1:-}"
@@ -596,7 +695,7 @@ 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$_MAINTAINER_GATE_INFO_JQ_DEFS"'
+    --argjson infopatterns "$info_patterns" "$_MAINTAINER_GATE_DISP_JQ_DEFS$_MAINTAINER_GATE_SCOPE_JQ_DEFS$_MAINTAINER_GATE_INFO_JQ_DEFS$_MAINTAINER_GATE_NO_FINDING_JQ_DEFS"'
       (.comments // []) as $all
       | [ $all[] | objects
         | (.author?.login // "" | tostring | if endswith("[bot]") then .[0:-5] else . end) as $lbare
@@ -608,6 +707,11 @@ check_maintainer_comments() {
         # unrecognised body, a bot with no pattern, a human, a User account
         # holding a bot login) is still evaluated.
         | select(info_status_clear | not)
+        # Drop a no-finding Codex or CodeRabbit summary: the bot reports, in its
+        # own structured state, that its review found nothing. It clears whether
+        # or not it is minimized and however often the bot has edited it, so a
+        # hands-off PR (no dev-lead to re-disposition each edit) keeps approval.
+        | select(no_finding_summary | not)
         | if ((.isMinimized // false) == true)
              and (((.minimizedReason // "") | ascii_downcase) == "resolved") | not
           then "block"