diff --git a/docs/architecture/adr/0012-comment-gate-exemption-set.md b/docs/architecture/adr/0012-comment-gate-exemption-set.md index 7fc34d930..0576570ab 100644 --- a/docs/architecture/adr/0012-comment-gate-exemption-set.md +++ b/docs/architecture/adr/0012-comment-gate-exemption-set.md @@ -56,6 +56,18 @@ 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 @@ -63,6 +75,11 @@ 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`), diff --git a/docs/pr-review-agent/maintainer-comment-gate.md b/docs/pr-review-agent/maintainer-comment-gate.md index 394c192b5..90a5c3fc7 100644 --- a/docs/pr-review-agent/maintainer-comment-gate.md +++ b/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) diff --git a/scripts/dev-lead-fix-reviews.sh b/scripts/dev-lead-fix-reviews.sh index fe0a6d1d6..fb073092c 100755 --- a/scripts/dev-lead-fix-reviews.sh +++ b/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) diff --git a/scripts/lib/bot-comment-retry.sh b/scripts/lib/bot-comment-retry.sh index c5c1c8d3e..374cd5e7b 100644 --- a/scripts/lib/bot-comment-retry.sh +++ b/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=` 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). diff --git a/scripts/lib/maintainer-comment-gate.sh b/scripts/lib/maintainer-comment-gate.sh index 4be4aad7f..be36c6df8 100755 --- a/scripts/lib/maintainer-comment-gate.sh +++ b/scripts/lib/maintainer-comment-gate.sh @@ -41,7 +41,9 @@ # β€’ from an OWNER/MEMBER/COLLABORATOR only: the explicit opt-out marker # ``; # β€’ 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\\s*")) 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(?:\\s*)?")) + and ([$b | scan("final_review_risk_label")] | length) == 1 + and ($b | test("")) + and ([$b | scan("final_review_risk_coverage")] | length) == 1 + and ($b | nf_json("") + | (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. 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,11 +615,12 @@ 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 ] @@ -545,6 +628,22 @@ maintainer_gate_open_comment_ids() { ' 2>/dev/null } +# maintainer_gate_no_finding_ids +# 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 [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" diff --git a/tests/dev-lead/unit/test_bot_comment_retry.bats b/tests/dev-lead/unit/test_bot_comment_retry.bats index 01fa76eb5..728f1bde0 100644 --- a/tests/dev-lead/unit/test_bot_comment_retry.bats +++ b/tests/dev-lead/unit/test_bot_comment_retry.bats @@ -162,6 +162,36 @@ spoof' 2026-10-01T23:30:00Z NONE)")" [ "$(jq 'length' <<< "$out")" = "0" ] } +# combsmith/.github-private PRs 2456/2467: a no-finding Codex or CodeRabbit summary +# needs no disposition (the gate clears it), so it never needs a fix-bot-comment +# pass either, however often the bot edits it. A summary with findings still does. +_NF_FIX="$SCRIPT_DIR/tests/fixtures" + +@test "bcr: a no-finding Codex or CodeRabbit summary is not a candidate, edited or not" { + local cx cr + cx="$(cat "$_NF_FIX/codex/pr2467-review-summary-no-findings.md")" + cr="$(cat "$_NF_FIX/coderabbit/pr2467-summary-minimal-ratelimited.md")" + out="$(_decide \ + "$(_bot IC_cx chatgpt-codex-connector "$cx" 2026-10-01T22:00:00Z 2026-10-01T23:40:00Z RESOLVED)" \ + "$(_bot IC_cr coderabbitai "$cr" 2026-10-01T22:00:00Z 2026-10-01T23:40:00Z)" \ + "$(_ours ' +Clean status.' 2026-10-01T23:00:00Z)")" + [ "$(jq 'length' <<< "$out")" = "0" ] +} + +@test "bcr: a Codex summary reporting findings, or a Low-risk CodeRabbit summary, still dispatches" { + local cx cr + cx="$(sed 's/"has_any_findings":false,"has_unresolved_findings":false,"status":"completed"},"security/"has_any_findings":true,"has_unresolved_findings":true,"status":"completed"},"security/' \ + "$_NF_FIX/codex/pr2467-review-summary-no-findings.md")" + cr="$(sed 's/\*\*βšͺ Minimal\*\* Β· up to/**πŸ”΅ Low** Β· up to/' "$_NF_FIX/coderabbit/pr2467-summary-minimal-ratelimited.md")" + [[ "$cx" == *'"has_any_findings":true'* ]] + [[ "$cr" == *'**πŸ”΅ Low** Β· up to'* ]] + out="$(_decide "$(_bot IC_cx chatgpt-codex-connector "$cx" 2026-10-01T22:00:00Z)")" + [ "$(_dispatches "$out")" = "1" ] + out="$(_decide "$(_bot IC_cr coderabbitai "$cr" 2026-10-01T22:00:00Z)")" + [ "$(_dispatches "$out")" = "1" ] +} + @test "bcr: a comment already minimized RESOLVED is not a candidate" { out="$(_decide "$(_bot IC_cr coderabbitai 'Walkthrough' 2026-10-01T23:05:43Z '' RESOLVED)")" [ "$(jq 'length' <<< "$out")" = "0" ] diff --git a/tests/dev-lead/unit/test_maintainer_comment_gate.bats b/tests/dev-lead/unit/test_maintainer_comment_gate.bats index 62398fb64..5647981b0 100644 --- a/tests/dev-lead/unit/test_maintainer_comment_gate.bats +++ b/tests/dev-lead/unit/test_maintainer_comment_gate.bats @@ -1085,3 +1085,307 @@ _stale_nodes() { _run_check "$(jq -c '{reviews:[], comments:.}' <<< "$after")" [ "$status" -eq 0 ] } + +# ──────────────────────────────────────────────────────────────────── +# No-finding summaries on a dev-lead:hands-off PR (combsmith/.github-private +# PRs 2456 and 2467, 2026-11-06). Codex and CodeRabbit edit ONE summary comment +# on every push, and SonarQube Cloud posts a fresh quality-gate notice. Under +# #2008 every edit needed a new dev-lead disposition; on a hands-off PR dev-lead +# writes none, so pr-review approved and then dismissed its own approval. A +# summary whose bot reports, in its own structured state, that the review found +# nothing now needs no disposition (no_finding_summary). Any other state, any +# finding-bearing section, or a non-Bot author still blocks. +# ──────────────────────────────────────────────────────────────────── + +_FIX="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)/../../fixtures" +_CODEX_NF_BODY="$(cat "$_FIX/codex/pr2467-review-summary-no-findings.md")" +_CR_2467_BODY="$(cat "$_FIX/coderabbit/pr2467-summary-minimal-ratelimited.md")" +_CR_2456_BODY="$(cat "$_FIX/coderabbit/pr2456-summary-minimal.md")" +_SONAR_2467_BODY="$(cat "$_FIX/sonarqubecloud/pr2467-quality-gate-passed.md")" +_CR_ARCH_FINDING=$'\n\n### Security Architecture Review\n\n**Security architecture risk:** _🟑 Moderate_ Β· up to `11929`\n\n**Retained concerns**\n- **Medium Β· security Β· inferred:** the lookup trusts an unvalidated name.\n\n' + +# _codex_body [security_review_json] β€” the live PR 2467 summary +# with its codex-review-state:v1 JSON replaced. +_CODEX_SEC_NONE='{"commit_sha":null,"has_any_findings":null,"has_unresolved_findings":null,"status":null}' +_codex_body() { + local cr="$1" sr="${2:-$_CODEX_SEC_NONE}" + jq -rn --arg b "$_CODEX_NF_BODY" --arg s "{\"code_review\":$cr,\"security_review\":$sr}" \ + '$b | sub("codex-review-state:v1 \\{[^\\n]*\\} -->"; "codex-review-state:v1 " + $s + " -->")' +} +# _cxs [commit_sha] β€” one +# review object of the state JSON. Values are JSON literals ("null", "false", …). +_cxs() { + jq -cn --argjson a "$1" --argjson u "$2" --argjson s "$3" \ + --argjson c "${4:-\"11929668fede1b15458100abc4347bb494eff259\"}" \ + '{commit_sha:$c, has_any_findings:$a, has_unresolved_findings:$u, status:$s}' +} + +# _sub β€” one jq sub() on a body. +_sub() { jq -rn --arg b "$1" --arg r "$2" --arg t "$3" '$b | sub($r; $t)'; } + +# _hands_off_pr β€” the PR 2467 shape at +# the moment pr-review dismissed its approval: all three bot comments edited or +# re-posted after the last disposition, and no dev-lead reply to cover them. The +# Codex summary is still minimized RESOLVED from an earlier pass. +_hands_off_pr() { + jq -cn --arg cx "$1" --arg cr "$2" --arg sq "$3" ' + def bot($id; $l; $b; $e; $m): + {id:$id, author:{login:$l, __typename:"Bot"}, authorAssociation:"NONE", body:$b, + createdAt:"2026-11-06T21:16:21Z", lastEditedAt:$e, isMinimized:$m, + minimizedReason:(if $m then "resolved" else null end)}; + {reviews:[], comments:[ + {id:"IC_req", author:{login:"don-petry", __typename:"User"}, authorAssociation:"OWNER", + body:"@donpetry-bot please review", createdAt:"2026-11-06T21:16:15Z", + lastEditedAt:null, isMinimized:false, minimizedReason:null}, + bot("IC_cx"; "chatgpt-codex-connector"; $cx; "2026-11-06T22:14:29Z"; true), + {id:"IC_disp", author:{login:"don-petry", __typename:"User"}, authorAssociation:"OWNER", + body:"Clean status.\n", + createdAt:"2026-11-06T21:50:02Z", lastEditedAt:null, isMinimized:false, minimizedReason:null}, + bot("IC_cr"; "coderabbitai"; $cr; "2026-11-06T22:12:56Z"; false), + bot("IC_sq"; "sonarqubecloud"; $sq; null; false) + ]}' +} + +# _expect_nf_open β€” gate β†’ 1 and the comment is a dev-lead candidate. +_expect_nf_open() { _expect_open "$@"; } + +@test "hands-off: the live PR 2467 snapshot (Codex, CodeRabbit, Sonar no-finding notices) keeps approval β†’ 0" { + _run_check "$(_hands_off_pr "$_CODEX_NF_BODY" "$_CR_2467_BODY" "$_SONAR_2467_BODY")" + [ "$status" -eq 0 ] + [[ "$output" == *"no undispositioned PR issue comments"* ]] +} + +@test "hands-off: the same snapshot leaves dev-lead nothing to disposition, re-open or chase" { + local comments + comments=$(_hands_off_pr "$_CODEX_NF_BODY" "$_CR_2467_BODY" "$_SONAR_2467_BODY" | jq -c .comments) + run bash -c "source '$GATE'; maintainer_gate_open_comment_ids \"\$1\" donpetry-bot" _ "$comments" + [ "$status" -eq 0 ] + [ -z "$output" ] + run bash -c "source '$GATE'; maintainer_gate_reopen_candidates \"\$1\" donpetry-bot" _ "$comments" + [ "$status" -eq 0 ] + [ "$output" = "[]" ] + run bash -c "source '$GATE'; maintainer_gate_stale_dispositions \"\$1\" donpetry-bot" _ "$comments" + [ "$status" -eq 0 ] + [ "$output" = "[]" ] +} + +# Both summaries minimized RESOLVED and edited after their only disposition: the +# #2008 re-open and stale lists would pick both up without the exemption. +@test "hands-off: RESOLVED Codex and CodeRabbit summaries edited after their disposition are not re-opened or chased" { + local comments + comments=$(_hands_off_pr "$_CODEX_NF_BODY" "$_CR_2467_BODY" "$_SONAR_2467_BODY" | jq -c ' + .comments |= map(if .id == "IC_cr" then .isMinimized = true | .minimizedReason = "resolved" else . end) + | .comments += [{id:"IC_disp_cr", author:{login:"don-petry", __typename:"User"}, authorAssociation:"OWNER", + body:"Clean summary.\n", + createdAt:"2026-11-06T21:50:30Z", lastEditedAt:null, isMinimized:false, minimizedReason:null}] + | .comments') + # Both summaries really are RESOLVED and edited after their disposition. + [ "$(jq '[.[] | select(.isMinimized and .lastEditedAt != null and .lastEditedAt > "2026-11-06T21:51:00Z")] | length' <<< "$comments")" = "2" ] + run bash -c "source '$GATE'; maintainer_gate_reopen_candidates \"\$1\" donpetry-bot" _ "$comments" + [ "$status" -eq 0 ] + [ "$output" = "[]" ] + run bash -c "source '$GATE'; maintainer_gate_stale_dispositions \"\$1\" donpetry-bot" _ "$comments" + [ "$status" -eq 0 ] + [ "$output" = "[]" ] + # dev-lead's resolver skips exactly these two when it builds its re-verify list. + run bash -c "source '$GATE'; maintainer_gate_no_finding_ids \"\$1\"" _ "$comments" + [ "$status" -eq 0 ] + [ "$(printf '%s\n' "$output" | sort | tr '\n' ' ')" = "IC_cr IC_cx " ] + _run_check "$(jq -c '{reviews:[], comments:.}' <<< "$comments")" + [ "$status" -eq 0 ] + # The same edited summaries WITH findings are still re-opened and chased. + local cx_find cr_low + cx_find="$(_codex_body "$(_cxs true true '"completed"')")" + cr_low="$(_sub "$_CR_2467_BODY" '\*\*βšͺ Minimal\*\* Β· up to' '**πŸ”΅ Low** Β· up to')" + comments=$(jq -c --arg cx "$cx_find" --arg cr "$cr_low" \ + 'map(if .id == "IC_cx" then .body = $cx elif .id == "IC_cr" then .body = $cr else . end)' <<< "$comments") + run bash -c "source '$GATE'; maintainer_gate_reopen_candidates \"\$1\" donpetry-bot" _ "$comments" + [ "$(jq -c 'sort' <<< "$output")" = '["IC_cr","IC_cx"]' ] + run bash -c "source '$GATE'; maintainer_gate_stale_dispositions \"\$1\" donpetry-bot" _ "$comments" + [ "$(jq -c '[.[].id] | sort' <<< "$output")" = '["IC_cr","IC_cx"]' ] + run bash -c "source '$GATE'; maintainer_gate_no_finding_ids \"\$1\"" _ "$comments" + [ -z "$output" ] +} + +@test "Wiring: dev-lead's re-verify list skips no-finding summaries" { + grep -q 'no_finding_ids=$(maintainer_gate_no_finding_ids "$all_comments"' "$SCRIPT_DIR/dev-lead-fix-reviews.sh" + grep -q '(($reopen + "\\n" + $nf) | split("\\n")) as $skip' "$SCRIPT_DIR/dev-lead-fix-reviews.sh" +} + +@test "hands-off: the same snapshot with a failed Sonar gate still withholds approval β†’ 1" { + _run_check "$(_hands_off_pr "$_CODEX_NF_BODY" "$_CR_2467_BODY" "$_SONAR_FAILED_BODY")" + [ "$status" -eq 1 ] +} + +@test "hands-off: the same snapshot with a Codex summary reporting findings still withholds approval β†’ 1" { + _run_check "$(_hands_off_pr "$(_codex_body "$(_cxs 'true' 'true' '"completed"')")" "$_CR_2467_BODY" "$_SONAR_2467_BODY")" + [ "$status" -eq 1 ] +} + +@test "hands-off: the same snapshot with a Low-risk CodeRabbit summary still withholds approval β†’ 1" { + _run_check "$(_hands_off_pr "$_CODEX_NF_BODY" "$(_sub "$_CR_2467_BODY" '\*\*βšͺ Minimal\*\* Β· up to' '**πŸ”΅ Low** Β· up to')" "$_SONAR_2467_BODY")" + [ "$status" -eq 1 ] +} + +# ── Codex: codex-review-state:v1 ───────────────────────────────────── + +# The builders must produce well-formed state, or a "still blocks" case below would +# pass on malformed JSON instead of on the state it means to test. +@test "Codex: the test body builder writes the state JSON it is given" { + local cr sr out + cr="$(_cxs true false '"completed"')" + sr="$(_cxs false false '"completed"')" + out="$(_codex_body "$cr" "$sr" | sed -n 2p | sed -E 's/^$/\1/')" + [ "$(jq -c . <<< "$out")" = "$(jq -cn --argjson c "$cr" --argjson s "$sr" '{code_review:$c, security_review:$s}')" ] + out="$(_codex_body "$cr" | sed -n 2p | sed -E 's/^$/\1/')" + [ "$(jq -c .security_review <<< "$out")" = "$_CODEX_SEC_NONE" ] +} + +@test "Codex: the live no-finding summary needs no disposition, unminimized or edited" { + _expect_clear chatgpt-codex-connector Bot "$_CODEX_NF_BODY" + _expect_clear 'chatgpt-codex-connector[bot]' Bot "$_CODEX_NF_BODY" + # Edited after its only disposition: #2008 would re-block a plain RESOLVED comment. + _run_check "$(_edit_json chatgpt-codex-connector "$_CODEX_NF_BODY" true "2026-10-01T20:35:00Z" "$(_disp 2026-10-01T19:23:54Z informational)" \ + | jq -c '.comments[0].author.__typename = "Bot"')" + [ "$status" -eq 0 ] +} + +@test "Codex: a security review that completed with no findings also clears" { + _expect_clear chatgpt-codex-connector Bot "$(_codex_body "$(_cxs 'false' 'false' '"completed"')" "$(_cxs 'false' 'false' '"completed"')")" +} + +@test "Codex: a code review with findings, resolved or not, still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'true' 'true' '"completed"')")" + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'true' 'false' '"completed"')")" + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'false' 'true' '"completed"')")" +} + +@test "Codex: a failed, running or unknown review still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'true' 'true' '"failed"')")" + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'false' 'false' '"in_progress"')")" + _expect_nf_open "$bot" "$(_codex_body '{"commit_sha":null,"has_any_findings":null,"has_unresolved_findings":null,"status":"completed"}')" + _expect_nf_open "$bot" "$(_codex_body "$(_cxs 'false' 'false' '"completed"' '"1192966"')")" + _expect_nf_open "$bot" "$(_codex_body "$(_cxs '"false"' 'false' '"completed"')")" +} + +@test "Codex: a security review with findings, or still running, still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' ok + ok="$(_cxs false false '"completed"')" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs 'true' 'true' '"completed"')")" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs 'null' 'null' '"in_progress"')")" + # The security_review key missing altogether is not "never ran". + _expect_nf_open "$bot" "$(jq -rn --arg b "$_CODEX_NF_BODY" '$b | sub(",\"security_review\":\\{[^}]*\\}"; "")')" +} + +@test "Codex: a null, empty or partial security review state still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' ok + ok="$(_cxs false false '"completed"')" + _expect_nf_open "$bot" "$(_codex_body "$ok" 'null')" + _expect_nf_open "$bot" "$(_codex_body "$ok" '{}')" + _expect_nf_open "$bot" "$(_codex_body "$ok" '{"status":null,"has_any_findings":null,"has_unresolved_findings":null}')" + _expect_nf_open "$bot" "$(_codex_body "$ok" '{"status":"completed","has_any_findings":false,"has_unresolved_findings":false}')" +} + +@test "Codex: a completed security review for another or an invalid commit still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' ok + ok="$(_cxs false false '"completed"')" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs false false '"completed"' '"32c068e010e99f7e4bf1fe0c7223479e01d609a0"')")" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs false false '"completed"' '"1192966"')")" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs false false '"completed"' 'null')")" + _expect_nf_open "$bot" "$(_codex_body "$ok" "$(_cxs false false '"completed"' '"11929668FEDE1B15458100ABC4347BB494EFF259"')")" +} + +@test "Codex: malformed, duplicated or misplaced state still needs a disposition" { + local bot='{"login":"chatgpt-codex-connector","__typename":"Bot"}' + _expect_nf_open "$bot" "$(_sub "$_CODEX_NF_BODY" '"status":"completed"\}' '"status":"completed"')" + _expect_nf_open "$bot" "$_CODEX_NF_BODY"$'\n'"$(printf '%s' "$_CODEX_NF_BODY" | sed -n 2p)" + _expect_nf_open "$bot" $'Earlier state, quoted:\n\n'"$_CODEX_NF_BODY" + _expect_nf_open "$bot" "$(_sub "$_CODEX_NF_BODY" 'codex-review-state:v1' 'codex-review-state:v2')" +} + +@test "Codex: a no-finding state beside a finding-bearing section still needs a disposition" { + _expect_nf_open '{"login":"chatgpt-codex-connector","__typename":"Bot"}' "$_CODEX_NF_BODY"$'\n\nActionable comments posted: 2' +} + +@test "Codex: the no-finding summary from a User account, another bot or no author type still needs a disposition" { + _expect_nf_open '{"login":"chatgpt-codex-connector","__typename":"User"}' "$_CODEX_NF_BODY" + _expect_nf_open '{"login":"chatgpt-codex-connector"}' "$_CODEX_NF_BODY" + _expect_nf_open '{"login":"coderabbitai","__typename":"Bot"}' "$_CODEX_NF_BODY" + _expect_nf_open '{"login":"github-actions","__typename":"Bot"}' "$_CODEX_NF_BODY" +} + +# ── CodeRabbit: final_review_risk_label / _coverage / pre-merge badges ── + +@test "CodeRabbit: the live PR 2467 (rate-limited) and PR 2456 Minimal summaries need no disposition" { + _expect_clear coderabbitai Bot "$_CR_2467_BODY" + _expect_clear 'coderabbitai[bot]' Bot "$_CR_2467_BODY" + _expect_clear coderabbitai Bot "$_CR_2456_BODY" + # Edited after its only disposition, still minimized RESOLVED. + _run_check "$(_edit_json coderabbitai "$_CR_2467_BODY" true "2026-10-01T20:35:00Z" "$(_disp 2026-10-01T19:23:54Z informational)" \ + | jq -c '.comments[0].author.__typename = "Bot"')" + [ "$status" -eq 0 ] +} + +@test "CodeRabbit: a Low, Moderate or High merge risk still needs a disposition" { + local bot='{"login":"coderabbitai","__typename":"Bot"}' level + for level in 'πŸ”΅ Low' '🟑 Moderate' 'πŸ”΄ High'; do + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" '\*\*βšͺ Minimal\*\* Β· up to' "**$level** Β· up to")" + done +} + +@test "CodeRabbit: a failed, warning or inconclusive pre-merge check still needs a disposition" { + local bot='{"login":"coderabbitai","__typename":"Bot"}' st + for st in failed warning inconclusive; do + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" 'pre-merge-checks-passed\.svg' "pre-merge-checks-$st.svg")" + done + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" 'pre-merge-checks-counter-passed-14' 'pre-merge-checks-counter-failed-14')" + # No pre-merge badge at all is not "all passed". + _expect_nf_open "$bot" "$(jq -rn --arg b "$_CR_2467_BODY" '$b | gsub("pre-merge-checks-[a-z0-9-]*\\.svg"; "x.svg")')" +} + +@test "CodeRabbit: coverage that is not a review, or unreadable, still needs a disposition" { + local bot='{"login":"coderabbitai","__typename":"Bot"}' + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" '"kind":"reviewed"' '"kind":"target_branch_merge_carry_forward"')" + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" '"kind":"reviewed"\}' '"kind":"reviewed"')" + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" '' '')" +} + +@test "CodeRabbit: a second risk label, or one outside its field, still needs a disposition" { + local bot='{"login":"coderabbitai","__typename":"Bot"}' + _expect_nf_open "$bot" "$_CR_2467_BODY"$'\n' + _expect_nf_open "$bot" "$(_sub "$_CR_2467_BODY" '' "$_CR_ARCH_FINDING")" + _expect_nf_open "$bot" "$_CR_2467_BODY"$'\n\nActionable comments posted: 1' +} + +@test "CodeRabbit: the Minimal summary from a User account, another bot or no author type still needs a disposition" { + _expect_nf_open '{"login":"coderabbitai","__typename":"User"}' "$_CR_2467_BODY" + _expect_nf_open '{"login":"coderabbitai"}' "$_CR_2467_BODY" + _expect_nf_open '{"login":"chatgpt-codex-connector","__typename":"Bot"}' "$_CR_2467_BODY" +} + +@test "CodeRabbit: the older fixtures without the structured risk label still need a disposition" { + local bot='{"login":"coderabbitai","__typename":"Bot"}' + _expect_nf_open "$bot" "$(cat "$CR_FIXTURES/summary-clean.md")" + _expect_nf_open "$bot" "$(cat "$CR_FIXTURES/pr2000-ratelimited-with-security-finding.md")" +} + +# ── SonarQube Cloud: the fresh per-push quality-gate notice ────────── + +@test "Sonar: the live PR 2467 'Quality Gate passed' notice needs no disposition" { + _expect_clear sonarqubecloud Bot "$_SONAR_2467_BODY" +} + +@test "Sonar: a passed gate reporting new issues, or a failed gate, still needs a disposition" { + local bot='{"login":"sonarqubecloud","__typename":"Bot"}' + _expect_nf_open "$bot" "$(_sub "$_SONAR_2467_BODY" '\[0 New issues\]' '[3 New issues]')" + _expect_nf_open "$bot" "$_SONAR_FAILED_BODY" + _expect_nf_open '{"login":"sonarqubecloud","__typename":"User"}' "$_SONAR_2467_BODY" +} diff --git a/tests/fixtures/coderabbit/pr2456-summary-minimal.md b/tests/fixtures/coderabbit/pr2456-summary-minimal.md new file mode 100644 index 000000000..929b845e0 --- /dev/null +++ b/tests/fixtures/coderabbit/pr2456-summary-minimal.md @@ -0,0 +1,159 @@ + + + + + +Explain Change β†’ + + + + +
+Summary +
+
+ + + +## Summary by CodeRabbit + +* **Bug Fixes** + * Clean status notices are exempted only when posted by a registered GitHub App and matching the expected status pattern. Notices from user accounts, unknown authors, or nonmatching comments remain subject to review. + * Author information is included when checking comment edit times, improving consistency when evaluating status notices. + +* **Documentation** + * Clarified the author-type requirements for clean status exemptions. + + +The clean info-status exemption now requires a GitHub `Bot` author, a registered login, a matching body pattern, and no finding-bearing section. The gate and candidate filter share this classifier. The GraphQL lookup and snapshot merge now include comment author types. + +**Changes: Comment gate exemption** + +|Layer / File(s)|Summary| +|:---|:---| +|**Fetch and merge comment author types**
`scripts/lib/maintainer-comment-gate.sh`|The GraphQL query requests each comment author's type. The snapshot merge adds the returned type to matching comments.| +|**Apply the shared clean-status rule**
`scripts/lib/maintainer-comment-gate.sh`, `docs/architecture/adr/0012-comment-gate-exemption-set.md`, `docs/pr-review-agent/maintainer-comment-gate.md`, `tests/dev-lead/unit/test_maintainer_comment_gate.bats`, `tests/dev-lead/unit/test_maintainer_resolve_comment.bats`|The gate and candidate filter use a shared classifier. Tests cover Bot, User, other, and missing author types, as well as matching and nonmatching comment bodies. Documentation and the SonarCloud fixture reflect the updated author requirement and status pattern.| + + + + + + + + + + +**Estimated code review effort:** 3 (Moderate) | ~20 minutes + + + +**Change:** Bug fix + + +### Sequence Diagram(s) + +```mermaid +sequenceDiagram + participant GitHubGraphQL + participant SnapshotMerge + participant InfoStatusClear + participant CommentGate + participant CandidateFilter + GitHubGraphQL->>SnapshotMerge: Return comment IDs, edit times, and author types + SnapshotMerge->>CommentGate: Add author types to matching comment snapshots + CommentGate->>InfoStatusClear: Check comment author and body + CandidateFilter->>InfoStatusClear: Check comment author and body +``` + +
+
+
+ + +
+Merge Risk: Minimal Β· up to 66ce1 +
+
+ + + + + +The exemption now requires a matching Bot author and fails closed when author type is unavailable. Additional query-selection coverage is recommended, but no merge-blocking issue remains. + + +
+
+
+ + +
+Pre-merge checks: Passed 5 +
+
+ +| Check name | Status | Explanation | +| :------------------------ | :--------------------------------------------------------------------------------------------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Title check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | The title clearly and concisely identifies the main change: restricting maintainer-gate info-status exemptions to Bot authors and body prefixes. | +| Docstring Coverage | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… | +| Linked Issues check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Check skipped because no linked issues were found for this pull request. | +| Out of Scope Changes check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Check skipped because no linked issues were found for this pull request. | +| Description check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | The description covers the required Problem, Risk, Test plan, Rollback, and Monitoring sections with specific implementation details and test results. The Summary section and the explicit N/A Interact… | + +
+Full details: Docstring Coverage +
+
+ +**Explanation** + +Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 3 files. (2 skipped: 2 unsupported.) + +
+
+
+ +
+Full details: Description check +
+
+ +**Explanation** + +The description covers the required Problem, Risk, Test plan, Rollback, and Monitoring sections with specific implementation details and test results. The Summary section and the explicit N/A Interaction contract declaration are missing, but the description remains substantially complete. + +
+
+
+ +
+
+
+ + + +- [ ] Autofix Β· Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts + + + + + +Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=combsmith/.github-private&utm_content=2456)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. + +
+Share +
+
+ +- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) +- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) +- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) +- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) + +
+
+
+ + \ No newline at end of file diff --git a/tests/fixtures/coderabbit/pr2467-summary-minimal-ratelimited.md b/tests/fixtures/coderabbit/pr2467-summary-minimal-ratelimited.md new file mode 100644 index 000000000..b9274e412 --- /dev/null +++ b/tests/fixtures/coderabbit/pr2467-summary-minimal-ratelimited.md @@ -0,0 +1,211 @@ + + + + + +Explain Change β†’ + + + + +> [!WARNING] +> ## Review limit reached +> +> You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. +> +> **Next included review available in 3 minutes.** +> +> [Check out review usage here](https://app.coderabbit.ai/dashboard/review-capacity?orgId=a994a7f2-c841-43f7-8a43-35243a386f9f). +> +>
+> View limit details +>
+>
+>
+> +> **Limit details:** You’ve used the included review currently available. +> +> [Learn how review limits work](https://docs.coderabbit.ai/management/rate-limits). +> +> **Review configuration:** +> +>
+> Run configuration +>
+>
+>
+> +> - **Source mode**: Auto +> - **Repository**: Unavailable +> - **Branch**: Unavailable +> - **File**: No configuration file from this repository applied +> - **Commit**: Unavailable +> - **Layers**: Organization UI +> - **Review profile**: ASSERTIVE +> - **Plan**: Advanced +> - **Run ID**: `3b57fdc4-eef2-46ad-83ae-54d729be333e` +> +>
+>
+>
+> +>
+> Commits +>
+>
+>
+> +> Reviewing files that changed from the base of the PR and between 32c068e010e99f7e4bf1fe0c7223479e01d609a0 and 11929668fede1b15458100abc4347bb494eff259. +> +>
+>
+>
+> +>
+> Files selected for processing: 2 +>
+>
+>
+> +> * `scripts/dev-lead-retry.sh` +> * `tests/dev-lead/unit/test_dev_lead_retry.bats` +> +>
+>
+>
+> +>
+>
+>
+ + + + + +
+Summary +
+
+
+ + + +## Summary by CodeRabbit + +* **Bug Fixes** + * CI failure reports now include the ID and URL for the latest failed check matching its name. + * Check names containing quotes or backslashes are matched correctly. + * If check-run retrieval or filtering fails, the report uses empty values for the run ID and URL. + + +The check-run lookup now filters a raw API response with `jq --arg`. Tests cover selection of the latest failed matching run, literal matching of names with quotes and backslashes, and inclusion of the run ID and URL in the retry payload. + +**Changes: Check-run lookup** + +|Layer / File(s)|Summary| +|:---|:---| +|**Lookup and retry payload**
`scripts/dev-lead-retry.sh`, `tests/dev-lead/unit/test_dev_lead_retry.bats`|`lookup_check_run_details` filters raw check-run JSON with `jq --arg` and returns details for the latest failed matching run. Tests cover literal matching and verify that the retry payload contains the run ID and URL.| + + + + + + + + + + +**Estimated code review effort:** 2 (Simple) | ~10 minutes + + + +**Change:** Bug fix + + +**Possibly related PRs** + +- [combsmith/.github-private#2462](https://github.com/combsmith/.github-private/pull/2462): Updates the API error fallback in `lookup_check_run_details`; this PR changes filtering in the same lookup. + +
+
+
+ + +
+Merge Risk: Minimal Β· previous review 32c06 +
+
+
+ + + + + +The change restores check-run details in retry payloads. Pagination remains an existing limitation, and the additional test suggestions are nonblocking; no introduced merge-blocking risk is established. + + +
+
+
+ + +
+Pre-merge checks: Passed 5 +
+
+
+ +| Check name | Status | Explanation | +| :------------------------ | :--------------------------------------------------------------------------------------------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Title check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | The title clearly describes the restored check run ID and URL in CI retry payloads. | +| Docstring Coverage | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… | +| Linked Issues check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Check skipped because no linked issues were found for this pull request. | +| Out of Scope Changes check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | Check skipped because no linked issues were found for this pull request. | +| Description check | ![Passed](https://storage.googleapis.com/coderabbit_public_assets/pre-merge-checks-passed.svg) | The description is mostly complete. It covers the problem, risk, test plan, rollback, and monitoring. The required Summary and Checklist sections are missing. | + +
+Full details: Docstring Coverage +
+
+
+ +**Explanation** + +Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. + +
+
+
+ +
+
+
+ + + +- [ ] Autofix Β· Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts + + + + + +Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=combsmith/.github-private&utm_content=2467)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. + +
+Share +
+
+
+ +- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) +- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) +- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) +- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) + +
+
+
+ + \ No newline at end of file diff --git a/tests/fixtures/codex/pr2467-review-summary-no-findings.md b/tests/fixtures/codex/pr2467-review-summary-no-findings.md new file mode 100644 index 000000000..a735a262e --- /dev/null +++ b/tests/fixtures/codex/pr2467-review-summary-no-findings.md @@ -0,0 +1,27 @@ + + + +
+Codex Review Summary: βœ… Completed + +This comment shows the latest Codex review activity on this pull request. + +| Review | Status | Commit | Review trigger | +| --- | --- | --- | --- | +| πŸ“ **Code Review** | βœ… **Completed** 2026-11-06T22:14:28.919935Z | `1192966` | New commits | + + + +
ℹ️ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review" or "@codex security review". + +Codex reacts with πŸ‘€ while any review is running, comments if it has suggestions, and reacts with πŸ‘ once all reviews finish with no findings. + +
+ +
\ No newline at end of file diff --git a/tests/fixtures/sonarqubecloud/pr2467-quality-gate-passed.md b/tests/fixtures/sonarqubecloud/pr2467-quality-gate-passed.md new file mode 100644 index 000000000..d5d48eeb9 --- /dev/null +++ b/tests/fixtures/sonarqubecloud/pr2467-quality-gate-passed.md @@ -0,0 +1,13 @@ +## [![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-private&pullRequest=2467) **Quality Gate passed** +Issues +![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=combsmith_.github-private&pullRequest=2467&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true) +![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=combsmith_.github-private&pullRequest=2467&issueStatuses=ACCEPTED) + +Measures +![](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-private&pullRequest=2467&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true) +![](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-private&pullRequest=2467&metric=new_coverage&view=list) +![](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-private&pullRequest=2467&metric=new_duplicated_lines_density&view=list) + + +[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=combsmith_.github-private&pullRequest=2467) +