feat: implement issue #773 — pr-review: rubber-duck tier broken under default claude engine — COPILOT_API_MODEL: unbound variable

prompts/cascade-action.md

@@ -50,6 +50,9 @@ cat > /tmp/cascade/review-body.txt << 'BODYEOF'
 ### Summary
 PLACEHOLDER_SUMMARY
 
+### Cross-engine agreement (if deep+duck)
+<If tier is deep+duck and agreement field exists, include this section>
+
 ### Findings
 PLACEHOLDER_FINDINGS_LIST
 

scripts/dev-lead-fix-ci.sh

@@ -187,6 +187,23 @@ ${details}"
   fi
 }
 
+post_exhaustion() {
+  local reason="$1"
+  local body="${EXHAUSTION_MARKER}
+## Dev-Lead Fix CI — exhausted
+
+This PR has had **${MAX_FAIL_ATTEMPTS}** consecutive engine failures (timeouts or errors). Automated CI fixing has been paused to avoid consuming further tokens.
+
+**Reason for last failure:** ${reason}
+
+To re-enable, delete this comment or push a new commit with a substantially different change."
+  if [ "${DEV_LEAD_DRY_RUN:-false}" = "true" ]; then
+    echo "[dry-run] would post exhaustion comment"
+  else
+    gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$body"
+  fi
+}
+
 main() {
   if [ -z "$PR_NUMBER" ] || [ -z "$HEAD_SHA" ]; then
     echo "::error::PR_NUMBER and HEAD_SHA are required"

scripts/engine.sh

@@ -30,6 +30,17 @@ set -euo pipefail
 REVIEW_ENGINE="${REVIEW_ENGINE:-claude}"
 export REVIEW_ENGINE
 
+# Cross-engine rubber-duck model (issue #773). The duck deliberately routes to
+# Copilot/o4-mini even when the primary engine is NOT copilot (e.g. the default
+# claude engine sets DUCK_ENGINE=copilot for adversarial diversity), so
+# copilot_chat() may dereference COPILOT_API_MODEL regardless of REVIEW_ENGINE.
+# Default it here — engine-agnostic — so the duck always has a model under
+# `set -u`. The `copilot)` arm in set_engine_config re-applies the same default
+# (idempotent) for the primary-engine path.
+DEFAULT_COPILOT_API_MODEL="openai/o4-mini"
+COPILOT_API_MODEL="${COPILOT_API_MODEL:-$DEFAULT_COPILOT_API_MODEL}"
+export COPILOT_API_MODEL
+
 # Per-tier timeouts (seconds). The job-level 60min cap is a backstop — without
 # per-tier timeouts a single hung model invocation burns the whole hour and
 # blocks every subsequent PR in the session.

scripts/list-prs.sh

@@ -24,6 +24,24 @@
 #   2. All other repos (priority 1)
 #   Within each priority tier, PRs are sorted oldest-first by createdAt.
 #
+# Filters:
+#   --draft=false       — skip work-in-progress PRs
+#
+# CI filtering is intentionally omitted here — review-one-pr.sh enforces it
+# per-PR as a second layer. Filtering by --checks success would exclude repos
+# with no CI configured (GitHub treats "no checks" as not matching --checks
+# success), causing their PRs to never enter the candidate pool.
+#
+# Self-authored PRs (PRs whose author is $BOT_USER) are excluded here, because
+# GitHub's GraphQL API rejects self-approval unconditionally — including such a
+# PR in the queue previously triggered a fatal session abort that starved every
+# subsequent candidate (see issue #96).
+#
+# Output ordering (stable, deterministic):
+#   1. .github and .github-private PRs first (priority 0)
+#   2. All other repos (priority 1)
+#   Within each priority tier, PRs are sorted oldest-first by createdAt.
+#
 # Output: one PR URL per line on stdout.
 
 set -euo pipefail

scripts/pr_review_health.sh

@@ -1,25 +1,23 @@
 #!/usr/bin/env bash
-# Daily health check for the PR Review Agent workflow.
+# Daily telemetry check for the PR Review Agent workflow.
 #
-# Fetches recent pr-review.yml run logs, feeds them to Claude for pattern
-# analysis, and writes a markdown report to pr_review_health_report.md.
-# Sets HAS_FAILURES=true in $GITHUB_ENV when failed runs are detected.
+# Fetches recent pr-review.yml run data, computes health metrics, and
+# writes a structured markdown report to both GITHUB_STEP_SUMMARY and
+# pr_review_health_report.md. Sets HAS_FAILURES=true in GITHUB_ENV when
+# failed runs are detected.
 #
 # Env vars consumed:
-#   GH_TOKEN              — primary (GitHub App; must have actions:read on this repo)
-#   GH_PAT_FALLBACK       — fallback PAT if App token lacks access
-#   CLAUDE_CODE_OAUTH_TOKEN — passed through to claude CLI
-#   LOOKBACK_DAYS         — days of history to consider (default: 1, ~24 hours)
-#                           Set higher to examine longer windows (e.g., LOOKBACK_DAYS=7 for weekly review)
-#   GITHUB_ENV            — written by Actions runner; used to export HAS_FAILURES
+#   GH_TOKEN      — must have actions:read on WORKFLOW_REPO
+#   LOOKBACK_DAYS — days of history to consider (default: 1)
+#   GITHUB_ENV    — written by Actions runner
+#   GITHUB_STEP_SUMMARY — written by Actions runner
 
 set -euo pipefail
 
 LOOKBACK_DAYS="${LOOKBACK_DAYS:-1}"
 WORKFLOW_REPO="${AGENT_REPO:-petry-projects/.github-private}"
 WORKFLOW_FILE="pr-review-trigger.yml"
 REPORT_FILE="pr_review_health_report.md"
-LOG_DIR="health_run_logs"
 TODAY=$(date -u +%Y-%m-%d)
 
 echo "=== PR Review Agent — Daily Health Check ==="
@@ -30,41 +28,42 @@ echo "  Date:         $TODAY"
 echo ""
 
 # ---------------------------------------------------------------------------
-# 0. Token selection — verify GH_TOKEN has access to workflow run logs
+# 0. Token selection
 # ---------------------------------------------------------------------------
 if ! gh api "repos/${WORKFLOW_REPO}/actions/workflows/${WORKFLOW_FILE}/runs?per_page=1" \
      >/dev/null 2>&1; then
   if [ -n "${GH_PAT_FALLBACK:-}" ]; then
-    echo "::warning::App token cannot access ${WORKFLOW_REPO} run logs — using GH_PAT_FALLBACK"
+    echo "::warning::GH_TOKEN cannot access ${WORKFLOW_REPO} — using GH_PAT_FALLBACK"
     export GH_TOKEN="$GH_PAT_FALLBACK"
   else
-    echo "::error::App token cannot access ${WORKFLOW_REPO} run logs and GH_PAT_FALLBACK is not set."
-    echo "::error::Grant the GitHub App access to ${WORKFLOW_REPO} or set the DON_PETRY_BOT_GH_PAT secret."
+    echo "::error::GH_TOKEN cannot access ${WORKFLOW_REPO} and GH_PAT_FALLBACK is not set."
     exit 1
   fi
 fi
 
 # ---------------------------------------------------------------------------
-# 1. Fetch recent run metadata
+# 1. Fetch run metadata
 # ---------------------------------------------------------------------------
-# GNU date: date -d "N days ago"; macOS: date -v-Nd
 CUTOFF=$(date -u -d "${LOOKBACK_DAYS} days ago" +%Y-%m-%dT%H:%M:%SZ 2>/dev/null \
   || date -u -v-"${LOOKBACK_DAYS}"d +%Y-%m-%dT%H:%M:%SZ)
 
-echo "Fetching all runs since: $CUTOFF"
-# Fetch with per_page=100 (GitHub API max) to capture all runs in the lookback window.
-# Time-based filtering ensures we get every run created at or after CUTOFF, regardless of quantity.
+echo "Fetching runs since: $CUTOFF"
+
 runs_json=$(gh api \
   "repos/${WORKFLOW_REPO}/actions/workflows/${WORKFLOW_FILE}/runs?per_page=100&created=>=${CUTOFF}" \
   --jq '.workflow_runs | map({
     id: .id,
+    run_number: .run_number,
     status: .status,
     conclusion: .conclusion,
     created_at: .created_at,
     html_url: .html_url,
-    run_number: .run_number
+    duration_s: ((.updated_at | fromdate) - (.created_at | fromdate))
   })' 2>/dev/null || echo '[]')
 
+# ---------------------------------------------------------------------------
+# 2. Compute aggregate stats
+# ---------------------------------------------------------------------------
 read -r total_runs failed_runs success_runs cancelled_runs < <(echo "$runs_json" | jq -r '
   [
     length,
@@ -73,56 +72,63 @@ read -r total_runs failed_runs success_runs cancelled_runs < <(echo "$runs_json"
     ([.[] | select(.conclusion == "cancelled")] | length)
   ] | @tsv')
 
-echo "  Total runs:   $total_runs"
-echo "  Successful:   $success_runs"
-echo "  Failed:       $failed_runs"
-echo "  Cancelled:    $cancelled_runs"
-echo ""
+echo "  Total:      $total_runs"
+echo "  Success:    $success_runs"
+echo "  Failed:     $failed_runs"
+echo "  Cancelled:  $cancelled_runs"
 
-# ---------------------------------------------------------------------------
-# 2. Early exit when no failures
-# ---------------------------------------------------------------------------
-if [ "$failed_runs" -eq 0 ]; then
-  echo "No failed runs in the last ${LOOKBACK_DAYS} days. Health check passed."
-  printf '# PR Review Agent Health Check — %s\n\nAll %d run(s) inspected over the last %d days succeeded. No action required.\n' \
-    "$TODAY" "$total_runs" "$LOOKBACK_DAYS" > "$REPORT_FILE"
-  exit 0
+if [ "$total_runs" -gt 0 ]; then
+  failure_rate=$(echo "scale=1; $failed_runs * 100 / $total_runs" | bc)
+else
+  failure_rate="0.0"
 fi
 
-# Export flag for workflow step condition
-[ -n "${GITHUB_ENV:-}" ] && echo "HAS_FAILURES=true" >> "$GITHUB_ENV"
+# Duration percentiles across all completed runs (computed but not surfaced in the
+# LLM prompt — reserved for future structured-report use).
+# shellcheck disable=SC2034
+read -r dur_min dur_p50 dur_p95 dur_max < <(echo "$runs_json" | jq -r '
+  [.[] | select(.conclusion != null and .duration_s > 0) | .duration_s] | sort |
+  if length == 0 then "0 0 0 0"
+  else . as $d | ($d | length) as $n |
+    "\($d | min) \($d[$n * 50 / 100 | floor]) \($d[$n * 95 / 100 | floor]) \($d | max)"
+  end')
 
 # ---------------------------------------------------------------------------
-# 3. Download logs for failed runs
+# 3. Helpers
 # ---------------------------------------------------------------------------
-mkdir -p "$LOG_DIR"
-failed_run_ids=$(echo "$runs_json" | jq -r '.[] | select(.conclusion == "failure") | .id')
-
-for run_id in $failed_run_ids; do
-  {
-    gh run view "$run_id" --repo "$WORKFLOW_REPO" --log 2>/dev/null \
-      | head -c 200000 \
-      > "${LOG_DIR}/run_${run_id}.txt" \
-      || echo "(log unavailable for run $run_id)" > "${LOG_DIR}/run_${run_id}.txt"
-  } &
-done
-wait
-
-# Surface log-retrieval failures so the operator knows the diagnosis may be incomplete.
-missing_logs=0
-for run_id in $failed_run_ids; do
-  if grep -q "^(log unavailable" "${LOG_DIR}/run_${run_id}.txt" 2>/dev/null; then
-    echo "::warning::Failed run $run_id: log could not be retrieved — diagnosis will be incomplete for this run"
-    missing_logs=$((missing_logs + 1))
+fmt_dur() {
+  local s=$1
+  if [ "$s" -ge 60 ]; then
+    printf '%dm%ds' $((s / 60)) $((s % 60))
+  else
+    printf '%ds' "$s"
   fi
-done
-if [ "$missing_logs" -gt 0 ]; then
-  echo "  $missing_logs of $failed_runs failed run log(s) could not be retrieved"
+}
+
+conclusion_icon() {
+  case "$1" in
+    success)   echo "✅" ;;
+    failure)   echo "❌" ;;
+    cancelled) echo "⚪" ;;
+    skipped)   echo "⏭️" ;;
+    *)         echo "⏳" ;;
+  esac
+}
+
+# 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
+  overall="CRITICAL"
+elif [ "$(echo "$failure_rate > 20" | bc)" -eq 1 ]; then
+  overall="DEGRADED"
+else
+  overall="WARNING"
 fi
-echo ""
 
 # ---------------------------------------------------------------------------
-# 4. Fetch workflow source for token/permission context
+# 4. Build report
 # ---------------------------------------------------------------------------
 [ -n "${GITHUB_ENV:-}" ] && {
   if [ "$failed_runs" -gt 0 ]; then
@@ -242,4 +248,4 @@ rm -f "$logs_file"
 
 echo ""
 echo "Report written to $REPORT_FILE ($(wc -c < "$REPORT_FILE") bytes)"
-echo "=== Health check complete ==="
+echo "=== Health check complete ==="

scripts/review-one-pr.sh

@@ -245,6 +245,24 @@ fi
   fi
 }
 
+# Skip when a human has requested changes, with two guards:
+#   1. FORCE_REVIEW bypasses the skip — mention-triggered runs always proceed so
+#      authors can request a re-review after addressing feedback.
+#   2. Only skip when a CHANGES_REQUESTED review targets the current head SHA.
+#      On repos that don't dismiss stale reviews, reviewDecision can stay
+#      CHANGES_REQUESTED after the author pushes new commits; in that case the
+#      review is stale and the cascade should re-engage with the updated code.
+if [ "$REVIEW_DECISION" = "CHANGES_REQUESTED" ] && [ "${FORCE_REVIEW:-false}" != "true" ]; then
+  CHANGES_REQUESTED_AT_HEAD=$(echo "$PR_SNAPSHOT" | jq -r --arg sha "$PR_HEAD_SHA" '
+    [.reviews[] | select(.state == "CHANGES_REQUESTED" and .commit.oid == $sha)]
+    | if length > 0 then "true" else "false" end
+  ')
+  if [ "$CHANGES_REQUESTED_AT_HEAD" = "true" ]; then
+    echo "    skip: changes requested at current head — awaiting author response before reviewing"
+    echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"skip\",\"reason\":\"changes-requested\"}"
+    exit 100
+  fi
+fi
 # Skip when a human has requested changes, with two guards:
 #   1. FORCE_REVIEW bypasses the skip — mention-triggered runs always proceed so
 #      authors can request a re-review after addressing feedback.