feat(#1894): answer "why isn't this PR approved?" from the real pr-review gate chain (+ why-not-approved agent)

.github/workflows/lint.yml

@@ -146,6 +146,7 @@
             tests/test_validate_cases.bats \
             tests/test_review_registry.bats \
             tests/test_reviewer_sources.bats \
+            tests/test_pr_approval_diagnostic.bats \
             tests/test_deep_specialist.bats \
             tests/test_pr_diff_dispatch.bats \
             tests/test_plan_review.bats \
@@ -310,7 +311,7 @@
     timeout-minutes: 5
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
         # NOTE: persist-credentials is intentionally left at the default (true) —
         # vci_resolve_reusable needs `git fetch origin` to authenticate against this
         # private repo when resolving same-repo channel tags. Setting
         # persist-credentials: false would silently push every such caller into the

scripts/lib/advisory-review-gate.sh

@@ -165,14 +165,20 @@ readonly YELLOW='\033[1;33m'
 readonly GREEN='\033[0;32m'
 readonly NC='\033[0m' # No Color
 
+# log_info <message...>
+#   Log informational message to stderr with advisory-gate prefix.
 log_info() {
   echo "[advisory-gate] $*" >&2
 }
 
+# log_warn <message...>
+#   Log warning message to stderr with yellow color and WARNING label.
 log_warn() {
   echo -e "${YELLOW}[advisory-gate] WARNING: $*${NC}" >&2
 }
 
+# log_success <message...>
+#   Log success message to stderr with green color.
 log_success() {
   echo -e "${GREEN}[advisory-gate] $*${NC}" >&2
 }
@@ -452,23 +458,11 @@ Advisory bots were rate-limited; auto-approval is withheld until they recover. p
   fi
 }
 
-# Instant (non-blocking) check of advisory bot status
-#
-# DESIGN: This uses a re-trigger pattern instead of blocking waits:
-# 1. On first pr-review trigger (check_suite completion): instant check
-# 2. If return 0: bots ready → approve immediately
-# 3. If return 1: bots not ready → skip (exit 100)
-# 4. When bots submit: pull_request_review event fires
-# 5. pr-review re-triggered → this check returns 0 → approve
-#
-# Returns:
-#   0 = All detected participating bots have submitted (ready to approve)
-#   1 = Waiting for bots (skip, will re-check on next review event)
-#
-# _record_partial_evidence <submitted> <required> <reason> — record approval on
-# PARTIAL advisory evidence so the miss-rate metric counts it (#1596). Gate runs
-# BEFORE the write, so with PARTIAL_EVIDENCE_STATE_FILE set we DEFER (record facts,
-# post NOTHING); post-pr-review.sh announces only after verifying the review (#1874).
+# _record_partial_evidence <submitted> <required> <reason>
+#   Record approval on PARTIAL advisory evidence so the miss-rate metric counts it
+#   (#1596). Gate runs BEFORE the write, so with PARTIAL_EVIDENCE_STATE_FILE set we
+#   DEFER (record facts, post NOTHING); post-pr-review.sh announces only after
+#   verifying the review (#1874).
 _record_partial_evidence() {
   if [ -n "${PARTIAL_EVIDENCE_STATE_FILE:-}" ]; then
     printf '%s %s %s\n' "$1" "$2" "$3" > "$PARTIAL_EVIDENCE_STATE_FILE" 2>/dev/null \
@@ -481,6 +475,12 @@ _record_partial_evidence() {
     || echo "::warning::partial-evidence marker post failed on ${PR_URL} — approval may be uncounted by the miss-rate metric (#1596)"
 }
 
+# check_advisory_reviews <pr_url>
+#   Instant (non-blocking) check of advisory bot review status. Returns 0 when all
+#   detected advisory bots have submitted their reviews (or the head-age/quiescence
+#   timeouts (#1193) for absent bots apply); returns 1 when still waiting; returns 2
+#   when the GitHub API query or jq parse fails — callers must fail closed, not treat
+#   it as "waiting".
 check_advisory_reviews() {
   local pr_url="${1:-}"
   if [[ -z "$pr_url" ]]; then
@@ -555,7 +555,11 @@ check_advisory_reviews() {
   effective_total=$((total_advisory_bots - num_unavailable))
   [[ "$effective_total" -lt 1 ]] && effective_total=1
   if [[ "$num_unavailable" -gt 0 ]]; then
-    log_info "${num_unavailable} advisory bot(s) rate-limited/unsupported — required set reduced to ${effective_total}/${total_advisory_bots}"
+    # Name WHICH bots dropped and why so the reduced denominator is observable — the
+    # registry total and effective total must never differ silently (#1894 AC #2).
+    local unavailable_named
+    unavailable_named=$(echo "$current_states" | jq -rs '[.[] | select(.state == "RATE_LIMITED" or .state == "UNSUPPORTED") | "\(.bot)=\(.state)"] | unique | join(", ")')
+    log_info "${num_unavailable} advisory bot(s) rate-limited/unsupported — required set reduced to ${effective_total}/${total_advisory_bots}; dropped: ${unavailable_named}"
   fi
 
   # Require the effective advisory set to submit before approving.

scripts/pr-approval-diagnostic.sh

@@ -0,0 +1,94 @@
+#!/usr/bin/env bash
+# pr-approval-diagnostic.sh — "why isn't this PR approved?" (#1894)
+#
+# Answers from the REAL gate chain, not a model of it: runs scripts/review-one-pr.sh
+# in diagnose mode (PR_REVIEW_DIAGNOSE=true — read-only, never forced, stops before
+# any model runs) and reports the single verdict that run reaches — which gate holds
+# the PR, and what would change the decision — together with the PR facts a reader
+# needs (review decision, approvals at the head, hold labels, merge state).
+#
+# The previous attempt (#1902) re-implemented each gate in a separate library and
+# drifted from the real ones (bot-comment handling, the unsupported-file regex,
+# check-run participation, the thread-gate fail-closed path). Running the gate chain
+# itself makes that drift impossible: a gate change is a diagnostic change.
+#
+# Usage: pr-approval-diagnostic.sh <pr-url> [--json]
+#   Needs the same environment a pr-review run needs to READ the PR (GH_TOKEN with
+#   repo read; BOT_USER defaults to donpetry-bot). Writes nothing to GitHub.
+# Exit: 0 diagnosed · 2 the gate chain produced no verdict (its log tail is printed)
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+PR_URL="${1:-}"
+FORMAT="markdown"
+[ "${2:-}" = "--json" ] && FORMAT="json"
+if [[ ! "$PR_URL" =~ ^https://github\.com/[^/]+/[^/]+/pull/[0-9]+$ ]]; then
+  echo "usage: pr-approval-diagnostic.sh https://github.com/<owner>/<repo>/pull/<n> [--json]" >&2
+  exit 2
+fi
+
+LOG="$(mktemp)"
+trap 'rm -f "$LOG"' EXIT
+
+set +e
+PR_REVIEW_DIAGNOSE=true GITHUB_STEP_SUMMARY="" bash "$SCRIPT_DIR/review-one-pr.sh" "$PR_URL" >"$LOG" 2>&1
+rc=$?
+set -e
+
+# The run's verdict is its LAST emit_verdict line (a JSON object with .decision).
+verdict=$(grep -E '^\{"pr":' "$LOG" | tail -n 1 || true)
+if [ -z "$verdict" ] || ! jq -e '.decision' <<<"$verdict" >/dev/null 2>&1; then
+  echo "pr-approval-diagnostic: the gate chain produced no verdict (exit $rc); last lines:" >&2
+  tail -n 20 "$LOG" >&2
+  exit 2
+fi
+
+facts=$(gh pr view "$PR_URL" --json state,isDraft,headRefOid,reviewDecision,mergeStateStatus,labels,reviews 2>/dev/null \
+  | jq -c --arg holds "needs-human-review dev-lead:needs-human dev-lead:hands-off initiative:hold" '
+      .headRefOid as $head
+      | {state, isDraft, reviewDecision, mergeStateStatus,
+         holds: [(.labels // [])[].name | select(. as $l | ($holds | split(" ") | index($l)))],
+         approvals_at_head: [(.reviews // [])[]
+            | select(.state == "APPROVED" and (.commit.oid // "") == $head)
+            | (.author.login // "?")] | unique}' 2>/dev/null || echo '{}')
+
+report=$(jq -cn --argjson v "$verdict" --argjson f "$facts" --arg log "$(grep -E '^    ' "$LOG" || true)" \
+  '$v + {facts: $f, gate_log: $log}')
+
+if [ "$FORMAT" = "json" ]; then
+  printf '%s\n' "$report"
+  exit 0
+fi
+
+jq -r '
+  def code: "`" + (. // "") + "`";
+  .facts as $f
+  | (if ($f.reviewDecision // "") == "APPROVED"
+     then "**Approved.** Merge state: " + (($f.mergeStateStatus // "unknown") | code) + "."
+     else "**Not approved** (review decision: " + (($f.reviewDecision // "none") | if . == "" then "none" else . end | code) + ")."
+     end) as $status
+  | [ "## pr-review approval diagnostic: " + .pr,
+      "",
+      $status,
+      "",
+      "| | |",
+      "|---|---|",
+      "| Head | " + ((.sha // "")[0:8] | code) + " |",
+      "| What pr-review would do now | " + (.decision | code) + " — " + (.reason | code) + " |",
+      "| What changes that | " + ((.would_change // "") | gsub("\\|"; "\\|")) + " |",
+      "| Hold labels | " + (if (($f.holds // []) | length) > 0 then ($f.holds | map(code) | join(", ")) else "none" end) + " |",
+      "| Approvals at head | " + (if (($f.approvals_at_head // []) | length) > 0 then ($f.approvals_at_head | join(", ")) else "none" end) + " |",
+      "",
+      (if .decision == "proceed"
+       then "No gate holds this PR: the next pr-review run reviews it, and that review decides."
+       else "This is the first gate that holds the PR, in the order a pr-review run checks them. Gates after it were not evaluated."
+       end),
+      "",
+      "<details><summary>Gate log</summary>",
+      "",
+      "```",
+      .gate_log,
+      "```",
+      "</details>"
+    ] | join("\n")' <<<"$report"

scripts/review-one-pr.sh

@@ -115,6 +115,18 @@ source "$SCRIPT_DIR/lib/carry-forward.sh"
 PR_URL="${1:?usage: review-one-pr.sh <pr-url>}"
 export PR_URL
 
+# Diagnose mode (#1894): answer "why isn't this PR approved?" by running the REAL
+# gate chain below — the same functions, in the same order, on the same live
+# state — and stopping before any model runs. It is read-only (DRY_RUN) and never
+# forced: a diagnosis must report what a normal run would do, so the human
+# break-glass and the orphan-rescue bypass are both off. The answer is the run's
+# single emit_verdict line (decision, reason, would_change). Used by
+# scripts/pr-approval-diagnostic.sh; never set by the review workflows.
+if [ "${PR_REVIEW_DIAGNOSE:-false}" = "true" ]; then
+  DRY_RUN=true FORCE_REVIEW=false FORCE_RE_REVIEW=false
+  export DRY_RUN FORCE_REVIEW FORCE_RE_REVIEW
+fi
+
 # emit_verdict <decision> <reason> <would_change>
 #   Emit THE single structured verdict line for a run that DECLINES to review this
 #   PR (skip / noop / escalate / error). It carries pr, head sha, decision, a
@@ -133,6 +145,13 @@ emit_verdict() {
     --arg reason "$2" \
     --arg would_change "$3" \
     '{pr:$pr, sha:$sha, decision:$decision, reason:$reason, would_change:$would_change}'
+  # #1894: also record it in the step summary, so "why not approved?" is answered
+  # where an operator looks, not only in the run log. Best-effort.
+  if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
+    local _sha="${PR_HEAD_SHA:-}"
+    printf -- '- **%s** at `%s`: `%s` (%s). Changes when: %s\n' \
+      "$PR_URL" "${_sha:0:8}" "$1" "$2" "$3" >> "$GITHUB_STEP_SUMMARY" 2>/dev/null || true
+  fi
 }
 
 echo "==> $PR_URL"
@@ -891,6 +910,23 @@ if [ "${DRY_RUN:-false}" != "true" ]; then
     fi
     exit 101
   fi
+elif [ "${PR_REVIEW_DIAGNOSE:-false}" = "true" ]; then
+  # Read-only: report an exhausted budget without escalating (enforce_pr_budget writes).
+  if _diag_events=$(gather_pr_automation_events \
+        "$(echo "$PR_URL" | sed -E 's|.*/pull/([0-9]+).*|\1|')" \
+        "$(echo "$PR_URL" | sed -E 's|https://github.com/([^/]+/[^/]+)/pull/.*|\1|')") \
+     && pr_budget_exhausted "$_diag_events"; then
+    emit_verdict escalate automation-budget-exhausted "a human interaction (comment or approval) resets the per-PR automation budget"
+    exit 100
+  fi
+fi
+
+# Diagnose mode stops here: every gate has passed, so a review run would now start
+# the model cascade, whose own verdict (approve / fix-request / escalate) decides.
+if [ "${PR_REVIEW_DIAGNOSE:-false}" = "true" ]; then
+  echo "    diagnose: every gate passes — a review run would start the model cascade now"
+  emit_verdict proceed gates-clear "nothing gates this PR; the next pr-review run reviews it and its verdict decides (a review event, a push, or the pr-review sweep starts that run)"
+  exit 0
 fi
 
 # Detect if the PR's repo org supports AI delegation for automated fix requests.

tests/dev-lead/unit/test_advisory_review_gate.bats

@@ -169,10 +169,12 @@ teardown() {
 @test "Advisory gate: script is minimal (non-blocking means fewer lines)" {
   local lines
   lines=$(wc -l < "$SCRIPT_DIR/lib/advisory-review-gate.sh")
-  # Budget raised 500→515 for the #1596 partial-evidence decision-point hooks. The
+  # Budget raised 500→515 for the #1596 partial-evidence decision-point hooks, then
+  # 515→520 for the #1894 AC #2 dropped-bot naming (four lines that make the reduced
+  # advisory denominator observable — which member fell out of "6 != 7" and why). The
   # heavy marker logic lives in scripts/lib/partial-evidence-marker.sh; only the
   # three irreducible timeout-branch hooks (deduplicated via _record_partial_evidence)
-  # are here. Raised 515→560 for the #1903 cubic registration (author-scoped
+  # are here. Raised 520→565 for the #1903 cubic registration (author-scoped
   # trial-ended detector, fallback list, notice bots list, pattern accessor, and
   # comprehensive test coverage for false positive protection). Raised 560→600 for
   # #2008: the section-aware CodeRabbit rate-limit scope (a shared jq def that both