fix: treat Claude quota refusals as non-fatal in PR review health report

.github/workflows/lint.yml

@@ -141,6 +141,7 @@
             tests/auto_rebase_health.bats \
             tests/pr_review_outcomes.bats \
             tests/pr_review_sweep_metrics.bats \
+            tests/pr_review_health.bats \
             tests/model_pricing.bats \
             tests/test_check_model_pins.bats \
             tests/test_check_bats_negation.bats \
@@ -366,7 +367,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/pr_review_health.sh

@@ -226,6 +226,7 @@ read -r dur_n dur_min dur_p50 dur_p95 dur_max < <(echo "$runs_json" | jq -r '
 # ---------------------------------------------------------------------------
 # 3. Helpers
 # ---------------------------------------------------------------------------
+# Format seconds as a human-readable duration (e.g., "5m30s" or "45s").
 fmt_dur() {
   local s=$1
   if [ "$s" -ge 60 ]; then
@@ -235,6 +236,24 @@ fmt_dur() {
   fi
 }
 
+# llm_quota_refused <file>... — 0 when a FAILED Claude call's captured output
+# reads as a quota / usage-limit refusal (#2446), e.g. "You've hit your weekly
+# limit · resets 4pm (UTC)" or an HTTP 429. Only consulted after a non-zero exit,
+# so a successful analysis that merely discusses rate limits is never matched.
+# Narrower than engine.sh's _rate_limit_pattern on purpose: this script does not
+# source engine.sh, and anything not matched here still fails the step.
+llm_quota_refused() {
+  # Detect if a failed Claude call was refused due to quota or rate-limiting.
+  local f files=()
+  for f in "$@"; do
+    [[ -s "$f" ]] && files+=("$f")
+  done
+  [[ "${#files[@]}" -gt 0 ]] || return 1
+  grep -qiE "hit your ([a-z]+ )?limit|reached your ([a-z]+ )?limit|[0-9]+-[a-z]+ limit reached|usage limit|too many requests|quota (exceeded|exhausted|reached)|out of (credits|tokens)|overloaded_error|([^0-9]|^)(429|529)([^0-9]|$)" \
+    "${files[@]}"
+}
+
+# Return an emoji icon representing the workflow conclusion status.
 conclusion_icon() {
   case "$1" in
     success)   echo "✅" ;;
@@ -246,7 +265,6 @@ conclusion_icon() {
 }
 
 # overall is the pre-computed status fed as a hint to the LLM prompt below.
-# shellcheck disable=SC2034
 if [ "$failed_runs" -eq 0 ]; then
   overall="HEALTHY"
 elif [ "$(echo "$failure_rate > 50" | bc)" -eq 1 ]; then
@@ -275,6 +293,12 @@ RUNS_SUMMARY=$(echo "$runs_json" | jq -r '
 ')
 LOG_DIR="${LOG_DIR:-}"
 
+logs_file=""
+llm_out=""
+llm_err=""
+# Registered before the first mktemp so every exit path (including a failed
+# second mktemp or a failed report append) removes whatever was created.
+trap 'rm -f "$logs_file" "$llm_out" "$llm_err"' EXIT
 logs_file=$(mktemp)
 # One jq pass over failed runs; append each log file in the same order
 while IFS=$'\t' read -r run_id run_meta; do
@@ -346,16 +370,20 @@ if [ "${total_sweep_ticks:-0}" -gt 0 ] && [ "$sweep_ticks" -lt "$total_sweep_tic
 fi
 
 echo "Invoking Claude for log analysis..."
-# Claude writes errors to stdout (not stderr), so they'd silently land in
-# $REPORT_FILE with the stdout redirect below.  Wrap in `if !` so a non-zero
-# exit surfaces the file contents to the Actions log before aborting.
-if ! claude --print --model "$(ai_model_for_family sonnet)" --no-session-persistence >> "$REPORT_FILE" <<PROMPT
+# Claude writes errors to stdout (not stderr), so the output is captured to temp
+# files and only appended to $REPORT_FILE on success — an error line must never
+# be published as if it were analysis.
+llm_out=$(mktemp)
+llm_err=$(mktemp)
+if claude --print --model "$(ai_model_for_family sonnet)" --no-session-persistence \
+  > "$llm_out" 2> "$llm_err" <<PROMPT
 You are analyzing GitHub Actions workflow run logs for the PR Review Agent.
 
 ## Context
 - Workflow: \`${WORKFLOW_FILE}\` in repo \`${WORKFLOW_REPO}\`
 - Analysis window: last ${LOOKBACK_DAYS} days (all runs fetched via pagination)
 - Report date: ${TODAY}
+- Pre-computed health status: \`${overall}\`
 - Total runs fetched: ${total_runs} | Successful: ${success_runs} | Failed: ${failed_runs} | Cancelled: ${cancelled_runs}
 - Run duration across ${dur_n} completed run(s): p50 $(fmt_dur "$dur_p50") | p95 $(fmt_dur "$dur_p95") (min $(fmt_dur "$dur_min") / max $(fmt_dur "$dur_max"))
 
@@ -420,12 +448,23 @@ Single line: \`Health: X/10 — <one-sentence verdict>\`
 Output ONLY the markdown report — no preamble or commentary outside the report sections.
 PROMPT
 then
+  cat "$llm_err" >&2
+  cat "$llm_out" >> "$REPORT_FILE"
+elif llm_quota_refused "$llm_out" "$llm_err"; then
+  # Quota / usage-limit refusal (#2446): the provider declined the call, the
+  # health check itself is fine. Keep the deterministic sections, say why the
+  # analysis is missing, and exit 0 so this step does not fail the job and skip
+  # the runaway / stall / merge-ready scans that follow (they make no model call).
+  echo "::warning::Claude log analysis skipped — the call was refused for quota / usage limit. Deterministic report sections are still published. CLI output: $(cat "$llm_out" "$llm_err" | head -c 300 | tr '\n' ' ')"
+  {
+    printf '## Log analysis skipped\n\n'
+    printf '_The Claude log-analysis call was refused for quota / usage limit, so this report carries only the deterministic sections above. The model analysis returns on the next run with budget._\n'
+  } >> "$REPORT_FILE"
+else
   echo "::error::Claude invocation failed. CLI output follows:"
-  cat "$REPORT_FILE" >&2
-  rm -f "$logs_file"
+  cat "$llm_out" "$llm_err" >&2
   exit 1
 fi
-rm -f "$logs_file"
 
 [ -n "${GITHUB_STEP_SUMMARY:-}" ] && [ -f "$REPORT_FILE" ] && cat "$REPORT_FILE" >> "$GITHUB_STEP_SUMMARY"
 

tests/pr_review_health.bats

@@ -0,0 +1,111 @@
+#!/usr/bin/env bats
+# Tests for scripts/pr_review_health.sh — how the Claude log-analysis call's
+# failure is handled (#2446). `gh` and `claude` are stubbed on PATH, so no
+# network I/O happens; the stubbed gh returns an empty run window.
+# Run locally: bats tests/pr_review_health.bats
+
+# Set up test environment: create stubs for gh and claude CLIs.
+setup() {
+  SCRIPT="${BATS_TEST_DIRNAME}/../scripts/pr_review_health.sh"
+  WORK="$(mktemp -d)"
+  STUB_BIN="${WORK}/bin"
+  mkdir -p "$STUB_BIN"
+
+  # gh stub: every run list is empty, workflow discovery finds no ingress
+  # workflow, and the workflow-source read fails (the script tolerates it).
+  cat > "${STUB_BIN}/gh" <<'GH'
+#!/usr/bin/env bash
+case "$*" in
+  *"/contents/"*) exit 1 ;;
+  *"actions/workflows?per_page"*) exit 0 ;;
+  *"created="*) echo '[]' ;;
+  *) echo '{}' ;;
+esac
+GH
+  chmod +x "${STUB_BIN}/gh"
+
+  export PATH="${STUB_BIN}:${PATH}"
+  export GITHUB_ENV="${WORK}/github_env"
+  export GITHUB_STEP_SUMMARY="${WORK}/step_summary"
+  : > "$GITHUB_ENV"
+  : > "$GITHUB_STEP_SUMMARY"
+  cd "$WORK" || return 1
+}
+
+# Clean up temporary test directories.
+teardown() {
+  rm -rf "$WORK"
+}
+
+# make_claude <exit_code> <stdout_text> [stderr_text]
+make_claude() {
+  {
+    printf '#!/usr/bin/env bash\ncat >/dev/null\n'
+    printf 'printf %%s\\\\n %q\n' "$2"
+    [ -n "${3:-}" ] && printf 'printf %%s\\\\n %q >&2\n' "$3"
+    printf 'exit %s\n' "$1"
+  } > "${STUB_BIN}/claude"
+  chmod +x "${STUB_BIN}/claude"
+}
+
+@test "quota refusal from Claude: warns, keeps the deterministic report, exits 0" {
+  make_claude 1 "You've hit your weekly limit · resets 4pm (UTC)"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *"::warning::"*"quota"* ]]
+  grep -q '## Convergence latency (deterministic)' pr_review_health_report.md
+  grep -q 'Log analysis skipped' pr_review_health_report.md
+  # The CLI's refusal line is not pasted into the report as if it were analysis.
+  run grep -c "hit your weekly limit" pr_review_health_report.md
+  [ "$output" = "0" ]
+  grep -q 'Log analysis skipped' "$GITHUB_STEP_SUMMARY"
+}
+
+@test "quota refusal reported on stderr is also recognised" {
+  make_claude 1 "" "API Error: 429 Too Many Requests"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  grep -q 'Log analysis skipped' pr_review_health_report.md
+  run grep -c "429 Too Many Requests" pr_review_health_report.md
+  [ "$output" = "0" ]
+}
+
+@test "documented 'reached your weekly limit' wording is recognised" {
+  make_claude 1 "You've reached your weekly limit · resets 4pm (UTC)"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  grep -q 'Log analysis skipped' pr_review_health_report.md
+}
+
+@test "'5-hour limit reached' wording is recognised" {
+  make_claude 1 "5-hour limit reached - resets 4pm"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  grep -q 'Log analysis skipped' pr_review_health_report.md
+}
+
+@test "non-quota Claude failure still fails the step" {
+  make_claude 1 "Error: model not found"
+  run bash "$SCRIPT"
+  [ "$status" -eq 1 ]
+  [[ "$output" == *"::error::Claude invocation failed"* ]]
+  [[ "$output" == *"Error: model not found"* ]]
+}
+
+@test "successful Claude call appends the analysis to the report" {
+  make_claude 0 "### 1. Executive Summary"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  grep -q '## Convergence latency (deterministic)' pr_review_health_report.md
+  grep -q '### 1. Executive Summary' pr_review_health_report.md
+  run grep -c 'Log analysis skipped' pr_review_health_report.md
+  [ "$output" = "0" ]
+}
+
+@test "a successful analysis that discusses rate limits is not treated as a refusal" {
+  make_claude 0 "| Engine rate limit (Claude or Copilot quota) | 0 | — |"
+  run bash "$SCRIPT"
+  [ "$status" -eq 0 ]
+  grep -q 'Engine rate limit' pr_review_health_report.md
+  [[ "$output" != *"::warning::"*"quota"* ]]
+}