From ff6ebbc985967182d45a8e4f4f11f8ec120255b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 00:55:49 +0000 Subject: [PATCH 1/5] fix(pr-review): unblock queue starvation from self-authored PRs (#96) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A self-authored PR sorted first in the candidate list and triggered "Can not approve your own pull request" — which the session-fatal abort treated like an engine error, skipping all 28 remaining PRs on every run. - list-prs.sh: filter out PRs authored by REVIEWER_USER at enumeration - post-pr-review.sh: catch the GraphQL self-approval error and exit 100 (no-op) instead of 1, so a stray self-PR can no longer abort the batch - engine.sh: gh copilot renamed --target to --agent; fix the rubber-duck invocations so tier-2 cross-engine review works again - review-one-pr.sh: stop appending a duplicate "0" to REVIEW_CYCLE when grep -c finds no markers under set -o pipefail (was breaking the cycle-cap integer comparison) https://claude.ai/code/session_01EacTxiHSUhR6kxppXpmxig --- scripts/engine.sh | 6 +++--- scripts/list-prs.sh | 15 +++++++++++---- scripts/post-pr-review.sh | 18 +++++++++++++++--- scripts/review-one-pr.sh | 9 ++++++++- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/scripts/engine.sh b/scripts/engine.sh index fec106fb4..1c6ecc473 100755 --- a/scripts/engine.sh +++ b/scripts/engine.sh @@ -116,7 +116,7 @@ run_triage() { # gh copilot is now a built-in; auth via GH_PAT (user token with Copilot subscription). # Model selection is not supported by gh copilot suggest — uses Copilot's default. ( export GH_TOKEN="$COPILOT_GITHUB_TOKEN" - timeout "$TRIAGE_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --target shell + timeout "$TRIAGE_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --agent shell ) || rc=$? ;; esac @@ -159,7 +159,7 @@ run_agentic() { # gh copilot is now a built-in; auth via GH_PAT (user token with Copilot subscription). # Model selection is not supported by gh copilot suggest — uses Copilot's default. ( export GH_TOKEN="$COPILOT_GITHUB_TOKEN" - timeout "$DEEP_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --target shell + timeout "$DEEP_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --agent shell ) ;; esac @@ -220,7 +220,7 @@ run_duck() { unset CLAUDE_CODE_OAUTH_TOKEN 2>/dev/null || true # gh copilot is now a built-in; auth via GH_PAT (user token with Copilot subscription). ( export GH_TOKEN="$COPILOT_GITHUB_TOKEN" - timeout "$DUCK_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --target shell + timeout "$DUCK_TIMEOUT_SEC" gh copilot suggest "$(cat "$prompt_file")" --agent shell ) ;; *) diff --git a/scripts/list-prs.sh b/scripts/list-prs.sh index 376b3a7b5..f3ec276d1 100755 --- a/scripts/list-prs.sh +++ b/scripts/list-prs.sh @@ -25,6 +25,13 @@ TARGET_ORG="${TARGET_ORG:-petry-projects}" all_prs="" +# Filter: exclude PRs authored by REVIEWER_USER. The workflow token is owned by +# REVIEWER_USER; GitHub's GraphQL API rejects self-approval unconditionally, so +# any self-authored PR is unreviewable by this runner and would otherwise burn +# session capacity (and previously triggered a fatal session abort — see issue +# #96). Filter at enumeration time so self-authored PRs never enter the queue. +JQ_NOT_SELF=".[] | select(.author.login != \"$REVIEWER_USER\") | .url" + # Get all repos in personal account and search each while IFS= read -r repo; do prs=$(gh search prs \ @@ -32,8 +39,8 @@ while IFS= read -r repo; do --repo "$repo" \ --draft=false \ --limit 100 \ - --json url \ - --jq '.[].url' 2>/dev/null || true) + --json url,author \ + --jq "$JQ_NOT_SELF" 2>/dev/null || true) all_prs="${all_prs}${prs}"$'\n' done < <(gh repo list "$REVIEWER_USER" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true) @@ -45,8 +52,8 @@ while IFS= read -r repo; do --draft=false \ --checks success \ --limit 100 \ - --json url \ - --jq '.[].url' 2>/dev/null || true) + --json url,author \ + --jq "$JQ_NOT_SELF" 2>/dev/null || true) all_prs="${all_prs}${prs}"$'\n' done < <(gh repo list "$TARGET_ORG" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true) diff --git a/scripts/post-pr-review.sh b/scripts/post-pr-review.sh index 654fadcf7..1942f23d9 100755 --- a/scripts/post-pr-review.sh +++ b/scripts/post-pr-review.sh @@ -177,13 +177,25 @@ if [ "$DECISION" = "approve" ]; then echo "$BODY" > "$BODY_FILE" echo "Posting APPROVED review..." - gh pr review "$PR_URL" --approve --body "$(cat "$BODY_FILE")" || { + REVIEW_ERR_FILE="/tmp/pr-review-err-$$.txt" + gh pr review "$PR_URL" --approve --body "$(cat "$BODY_FILE")" 2>"$REVIEW_ERR_FILE" || { rc=$? + review_err=$(cat "$REVIEW_ERR_FILE" 2>/dev/null || true) + cat "$REVIEW_ERR_FILE" >&2 2>/dev/null || true + rm -f "$BODY_FILE" "$REVIEW_ERR_FILE" + # Self-approval is a permanent, PR-specific constraint — never the runner's + # fault and never recoverable on retry. Exit 100 (no-op sentinel) so the + # workflow loop skips this PR without aborting the rest of the session. + # See issue #96: a single self-authored PR at the top of the queue + # previously starved every batch. + if echo "$review_err" | grep -qiE 'Can not approve your own pull request'; then + echo "::warning::Cannot self-approve $PR_URL — skipping (exit 100)" + exit 100 + fi echo "ERROR: gh pr review failed with exit code $rc" - rm -f "$BODY_FILE" exit 1 } - rm -f "$BODY_FILE" + rm -f "$BODY_FILE" "$REVIEW_ERR_FILE" # Dismiss prior agent reviews / collapse prior agent comments now that the # newest review has landed. Best-effort: failures here don't break the run. diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index 7794ad452..dd11424d6 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -119,7 +119,14 @@ PR_BODIES=$( gh pr view "$PR_URL" --json reviews,comments \ --jq '((.reviews // []) + (.comments // [])) | .[].body | select(. != null)' 2>/dev/null || true ) -REVIEW_CYCLE=$(echo "$PR_BODIES" | grep -cE '` so # subsequent runs detect the marker and no-op without spamming. -if echo "$PR_BODIES" | grep -qE ''; then +if printf '%s\n' "$PR_BODIES" | grep -qE ''; then echo " noop: human-escalation marker present — cascade already capped" echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"noop\",\"reason\":\"human-escalated\"}" exit 100 From dc6a6c61674e302f0965e1b9f196273a245bc213 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 18:04:06 +0000 Subject: [PATCH 3/5] refactor(pr-review): collapse to single BOT_USER, route escalations via CODEOWNERS Drop AGENT_USER and REVIEWER_USER. The workflow now has one identity: BOT_USER (default don-petry-bot), which both owns the repos to scan and gets filtered out as the self-approval blocker. Human escalation no longer hard-codes a single reviewer; instead, scripts/request-codeowners- review.sh parses CODEOWNERS in the PR's repo and requests review from every @user / @org/team mention. - list-prs.sh: BOT_USER for both gh-repo-list and self-author filter - review-one-pr.sh, post-pr-review.sh: replace --user "$REVIEWER_USER" with the CODEOWNERS helper - repair-pr-approvals.sh: same gh-repo-list switch - pr-review.yml: drop AGENT_USER/REVIEWER_USER, set BOT_USER default to don-petry-bot - AGENT.md: updated guideline to describe single-identity model https://claude.ai/code/session_01EacTxiHSUhR6kxppXpmxig --- .github/workflows/pr-review.yml | 13 +++---- AGENT.md | 10 ++--- scripts/list-prs.sh | 37 ++++++++----------- scripts/post-pr-review.sh | 5 ++- scripts/repair-pr-approvals.sh | 2 +- scripts/request-codeowners-review.sh | 55 ++++++++++++++++++++++++++++ scripts/review-one-pr.sh | 4 +- 7 files changed, 87 insertions(+), 39 deletions(-) create mode 100755 scripts/request-codeowners-review.sh diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 367d7639f..e5a32daa6 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -43,14 +43,13 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 env: - REVIEWER_USER: ${{ vars.REVIEWER_USER || 'don-petry' }} + # Single bot identity. GH_PAT_WORKFLOWS authenticates as BOT_USER; that + # account owns/has-access-to the personal repos scanned by list-prs.sh, + # signs the cascade's escalation comments, and is filtered out of the + # candidate queue (it can't approve its own PRs). Human reviewers are + # routed via CODEOWNERS at escalation time, not pinned via env. + BOT_USER: ${{ vars.BOT_USER || 'don-petry-bot' }} TARGET_ORG: ${{ vars.TARGET_ORG || 'petry-projects' }} - BOT_USER: ${{ vars.BOT_USER || 'petry-review-bot' }} - # GitHub identity that GH_PAT_WORKFLOWS authenticates as. Distinct from - # REVIEWER_USER (the human reviewer): the agent runs as a bot account - # so it can review the human's PRs without hitting the self-approval - # block. PRs authored by AGENT_USER are filtered out at enumeration. - AGENT_USER: ${{ vars.AGENT_USER || 'don-petry-bot' }} AGENT_REPO: ${{ github.repository }} # Review engine: "claude" or "copilot". Controls which CLI and models are used. # gh variable set REVIEW_ENGINE --body copilot --repo petry-projects/.github-private diff --git a/AGENT.md b/AGENT.md index 2294c23c5..8ee9f0d6c 100644 --- a/AGENT.md +++ b/AGENT.md @@ -14,11 +14,11 @@ and **Copilot**. 1a. **@mention** — comment `@petry-review-bot` on any PR to trigger an immediate review, bypassing the hourly schedule. See [Mention-triggered reviews](#mention-triggered-reviews). 2. **Enumerate** — `scripts/list-prs.sh` queries GitHub for open PRs across - every repo the PAT can see, **excluding PRs authored by `AGENT_USER`** - (the workflow's authenticated identity, e.g. `don-petry-bot`). GitHub's - GraphQL API rejects self-approval unconditionally, so self-authored PRs - are unreviewable and would otherwise starve the queue. Output: one URL - per line. + every repo the PAT can see (the bot's personal account plus `TARGET_ORG`), + **excluding PRs authored by `BOT_USER`** (the workflow's authenticated + identity, default `don-petry-bot`). GitHub's GraphQL API rejects + self-approval unconditionally, so self-authored PRs are unreviewable and + would otherwise starve the queue. Output: one URL per line. 3. **Per-PR review** — `scripts/review-one-pr.sh` runs a cascading review where each tier only fires if the previous one flagged concerns: diff --git a/scripts/list-prs.sh b/scripts/list-prs.sh index d0e032771..9d79d3ece 100755 --- a/scripts/list-prs.sh +++ b/scripts/list-prs.sh @@ -2,7 +2,7 @@ # Enumerate open, non-draft PRs the agent should consider reviewing. # # Searches across two namespaces: -# 1. All open PRs in repos owned by $REVIEWER_USER (personal account) +# 1. All open PRs in repos owned by $BOT_USER (the bot's personal account) # 2. All open PRs in repos owned by $TARGET_ORG (organization) # # Filters: @@ -12,43 +12,36 @@ # never consume a review slot. review-one-pr.sh also # enforces this per-PR as a second layer of defence. # -# Note: Uses repo enumeration instead of @me/@review-requested, which don't work -# with GitHub App tokens (app tokens have no user identity). +# 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: one PR URL per line on stdout. set -euo pipefail -# Configurable via environment / repo variables -REVIEWER_USER="${REVIEWER_USER:-don-petry}" +# Configurable via environment / repo variables. BOT_USER is the GitHub +# identity the workflow PAT authenticates as — both the queue scope (which +# repos to scan) and the self-approval filter use it. +BOT_USER="${BOT_USER:-don-petry-bot}" TARGET_ORG="${TARGET_ORG:-petry-projects}" -# AGENT_USER is the GitHub identity the workflow PAT belongs to (typically a -# bot account, e.g. don-petry-bot). It is distinct from REVIEWER_USER, which -# is the human who owns the personal repos being scanned and to whom escalations -# are routed. Self-authored PRs by AGENT_USER are filtered out below. -AGENT_USER="${AGENT_USER:-don-petry-bot}" -# Reject AGENT_USER values that aren't valid GitHub usernames before +# Reject BOT_USER values that aren't valid GitHub usernames before # interpolating into a jq program. GitHub usernames are 1–39 chars of # [A-Za-z0-9-] and may not start or end with a hyphen. Anything else is # either a misconfiguration or an injection attempt — fail loud rather # than silently dropping PRs. -if ! [[ "$AGENT_USER" =~ ^[A-Za-z0-9](-?[A-Za-z0-9]){0,38}$ ]]; then - echo "::error::AGENT_USER='$AGENT_USER' is not a valid GitHub username" >&2 +if ! [[ "$BOT_USER" =~ ^[A-Za-z0-9](-?[A-Za-z0-9]){0,38}$ ]]; then + echo "::error::BOT_USER='$BOT_USER' is not a valid GitHub username" >&2 exit 1 fi all_prs="" -# Filter: exclude PRs authored by AGENT_USER. The workflow PAT authenticates -# as AGENT_USER; GitHub's GraphQL API rejects self-approval unconditionally, -# so any PR authored by the agent is unreviewable by this runner and would -# otherwise burn session capacity (and previously triggered a fatal session -# abort — see issue #96). Filter at enumeration time so self-authored PRs -# never enter the queue. -JQ_NOT_SELF=".[] | select(.author.login != \"$AGENT_USER\") | .url" +JQ_NOT_SELF=".[] | select(.author.login != \"$BOT_USER\") | .url" -# Get all repos in personal account and search each +# Get all repos in bot's personal account and search each while IFS= read -r repo; do prs=$(gh search prs \ --state open \ @@ -58,7 +51,7 @@ while IFS= read -r repo; do --json url,author \ --jq "$JQ_NOT_SELF" 2>/dev/null || true) all_prs="${all_prs}${prs}"$'\n' -done < <(gh repo list "$REVIEWER_USER" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true) +done < <(gh repo list "$BOT_USER" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true) # Get all repos in org and search each (require passing checks) while IFS= read -r repo; do diff --git a/scripts/post-pr-review.sh b/scripts/post-pr-review.sh index 1942f23d9..78ccc5186 100755 --- a/scripts/post-pr-review.sh +++ b/scripts/post-pr-review.sh @@ -285,10 +285,11 @@ COMMENT_END # has landed. A new fix-request also invalidates any prior approval. mark_prior_agent_items_obsolete "$PR_URL" else - # Escalate to human + # Escalate to human via CODEOWNERS — avoid hard-coding a single reviewer. echo "Escalating to human review..." gh pr edit "$PR_URL" --add-label needs-human-review 2>/dev/null || true - gh pr request-review "$PR_URL" --user "${REVIEWER_USER:-don-petry}" 2>/dev/null || true + SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + bash "$SCRIPT_DIR/request-codeowners-review.sh" "$PR_URL" || true fi fi diff --git a/scripts/repair-pr-approvals.sh b/scripts/repair-pr-approvals.sh index afbe65e5f..5ea97261c 100644 --- a/scripts/repair-pr-approvals.sh +++ b/scripts/repair-pr-approvals.sh @@ -81,7 +81,7 @@ while IFS= read -r repo; do done < <(gh pr list --repo "$repo" --state open --json url,number,comments --limit 100 2>/dev/null | jq -r '.[] | select(.comments | length > 0) | {url, number} | "\(.url)|\(.number)"') done < <( { - gh repo list "${REVIEWER_USER:-don-petry}" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true + gh repo list "${BOT_USER:-don-petry-bot}" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true gh repo list "${TARGET_ORG:-petry-projects}" --json nameWithOwner --jq '.[].nameWithOwner' 2>/dev/null || true } | sort -u ) diff --git a/scripts/request-codeowners-review.sh b/scripts/request-codeowners-review.sh new file mode 100755 index 000000000..a05fd3b6e --- /dev/null +++ b/scripts/request-codeowners-review.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Request review from every CODEOWNERS entry in a PR's repository. +# +# Inputs: +# $1 — PR URL +# +# Reads CODEOWNERS from .github/CODEOWNERS, CODEOWNERS, or docs/CODEOWNERS in +# the PR's repo. Extracts every @user / @org/team mention (path-pattern rules +# are not parsed — for an escalation we want every owner notified, not just +# the ones whose patterns match the diff). Requests review from each entry. +# +# Best-effort: every individual request-review call is tolerated to fail. The +# whole script also exits 0 unconditionally so the caller can chain it with +# other escalation steps. + +set -uo pipefail + +PR_URL="${1:?usage: request-codeowners-review.sh }" +OWNER_REPO=$(echo "$PR_URL" | sed -E 's|.*/([^/]+)/([^/]+)/pull/.*|\1/\2|') + +CODEOWNERS="" +for path in .github/CODEOWNERS CODEOWNERS docs/CODEOWNERS; do + encoded=$(gh api "repos/$OWNER_REPO/contents/$path" --jq '.content' 2>/dev/null) || continue + [ -z "$encoded" ] && continue + decoded=$(printf '%s' "$encoded" | base64 -d 2>/dev/null) || continue + [ -n "$decoded" ] && { CODEOWNERS="$decoded"; break; } +done + +if [ -z "$CODEOWNERS" ]; then + echo " CODEOWNERS not found in $OWNER_REPO — relying on needs-human-review label only" + exit 0 +fi + +# Strip comments, then collect every @user and @org/team mention. +mentions=$(printf '%s\n' "$CODEOWNERS" \ + | sed 's/#.*//' \ + | grep -oE '@[A-Za-z0-9_-]+(/[A-Za-z0-9_-]+)?' \ + | sed 's|^@||' \ + | sort -u) + +if [ -z "$mentions" ]; then + echo " CODEOWNERS in $OWNER_REPO has no @mentions" + exit 0 +fi + +while IFS= read -r entry; do + [ -z "$entry" ] && continue + if gh pr edit "$PR_URL" --add-reviewer "$entry" 2>/dev/null; then + echo " requested review from @$entry" + else + echo " warn: failed to request review from @$entry (insufficient scope or unknown user/team?)" + fi +done <<< "$mentions" + +exit 0 diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index 4ad94658a..5d18c31a0 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -160,11 +160,11 @@ This PR has been through $REVIEW_CYCLE automated review cycles (cap: $MAX_CYCLES Please take a look manually, or close this PR if it's no longer needed. Once a human review resolves the situation, remove the \`needs-human-review\` label and the cascade can be re-engaged on the next push. -_Posted by the ${BOT_USER:-petry-review-bot} PR-review cascade._ +_Posted by the ${BOT_USER:-don-petry-bot} PR-review cascade._ ESCALATION_END gh pr comment "$PR_URL" --body-file "$ESCALATION_BODY" || echo " warn: gh pr comment failed — escalation marker not posted; will retry next cycle" gh pr edit "$PR_URL" --add-label needs-human-review 2>/dev/null || true - gh pr request-review "$PR_URL" --user "${REVIEWER_USER:-don-petry}" 2>/dev/null || true + bash "$SCRIPT_DIR/request-codeowners-review.sh" "$PR_URL" || true rm -f "$ESCALATION_BODY" fi echo "{\"pr\":\"$PR_URL\",\"sha\":\"$PR_HEAD_SHA\",\"decision\":\"escalate\",\"reason\":\"max-cycles-reached\"}" From 07f086659e790e93bac647308ead1529e496763d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 18:12:16 +0000 Subject: [PATCH 4/5] refactor(pr-review): extract batch loop, hoist env, drop redundant step Workflow YAML drops from 231 to 111 lines by moving the per-PR review loop into scripts/review-batch.sh. The script can be syntax-checked and exercised locally; the workflow now just wires env vars and dispatches. - Hoist GH_TOKEN, MAX_PRS, CANDIDATE_LIMIT to job-level env (were repeated on individual steps). - Inline `gh auth status` into the install step; remove the standalone Verify auth step (its only output was a one-line auth dump). - Drop the `if: steps.list.outputs.count != '0'` guard and the step's `id`/output: review-batch.sh handles empty input as a no-op. - Collapse the duplicated summary-string branch in the review loop into a single template with a conditional fragment. No behavior change. https://claude.ai/code/session_01EacTxiHSUhR6kxppXpmxig --- .github/workflows/pr-review.yml | 149 ++++---------------------------- scripts/review-batch.sh | 114 ++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 134 deletions(-) create mode 100755 scripts/review-batch.sh diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index e5a32daa6..056f154c2 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -43,6 +43,8 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 env: + # Auth for every gh / script call in this job. The PAT must belong to BOT_USER. + GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} # Single bot identity. GH_PAT_WORKFLOWS authenticates as BOT_USER; that # account owns/has-access-to the personal repos scanned by list-prs.sh, # signs the cascade's escalation comments, and is filtered out of the @@ -52,28 +54,19 @@ jobs: TARGET_ORG: ${{ vars.TARGET_ORG || 'petry-projects' }} AGENT_REPO: ${{ github.repository }} # Review engine: "claude" or "copilot". Controls which CLI and models are used. - # gh variable set REVIEW_ENGINE --body copilot --repo petry-projects/.github-private REVIEW_ENGINE: ${{ vars.REVIEW_ENGINE || 'claude' }} - # Claude engine auth — routes through Max plan subscription. - # Generate with `claude setup-token`, store as repo secret. CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - # Copilot engine auth — reuses the existing GH_PAT (personal account token with Copilot subscription). - # No new secret needed: gh secret set GH_PAT is already present in this repo. + # Copilot engine reuses GH_PAT (a separate user PAT with Copilot subscription). COPILOT_GITHUB_TOKEN: ${{ secrets.GH_PAT }} - # Default to dry-run unless the repo variable LIVE_MODE is set to 'true'. - # workflow_dispatch can override via dry_run input. - # repository_dispatch (mention trigger) always runs live — the human explicitly asked. - # To go live: gh variable set LIVE_MODE --body true --repo petry-projects/.github-private + # Default to dry-run. workflow_dispatch can override; repository_dispatch + # (mention) always runs live; vars.LIVE_MODE=true switches scheduled runs live. DRY_RUN: ${{ github.event_name == 'repository_dispatch' && 'false' || inputs.dry_run || (vars.LIVE_MODE == 'true' && 'false' || 'true') }} PR_URL_OVERRIDE: ${{ inputs.pr_url || github.event.client_payload.pr_url }} - # Comma-separated list of GitHub orgs where AI delegation is enabled. - # When findings exist, the agent posts fix-request comments for AI to act on. - # gh variable set DELEGATION_ORGS --body "petry-projects,don-petry" --repo petry-projects/.github-private DELEGATION_ORGS: ${{ vars.DELEGATION_ORGS || vars.CLAUDE_ORGS || '' }} - # Max review cycles before stopping AI delegation and escalating to human. MAX_REVIEW_CYCLES: ${{ vars.MAX_REVIEW_CYCLES || '3' }} - # When true, bypass idempotency check so an @mention always triggers a fresh review. - # repository_dispatch always forces a review; workflow_dispatch respects the input. + MAX_PRS: ${{ vars.MAX_PRS || '10' }} + CANDIDATE_LIMIT: ${{ vars.CANDIDATE_LIMIT || '100' }} + # Mention-triggered reviews always force; workflow_dispatch respects the input. FORCE_REVIEW: ${{ github.event_name == 'repository_dispatch' && 'true' || inputs.force_review || 'false' }} steps: @@ -81,15 +74,12 @@ jobs: uses: actions/checkout@v5 - name: Install review engine CLIs - env: - GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} run: | - # Primary engine must install successfully; duck engine is best-effort. + set -euo pipefail + gh auth status case "$REVIEW_ENGINE" in claude) npm install -g @anthropic-ai/claude-code - # gh copilot is now a built-in in modern gh CLI — no extension install needed. - # Verify the built-in is accessible with the user token (GH_PAT). if ! env GH_TOKEN="$COPILOT_GITHUB_TOKEN" gh copilot --version > /dev/null 2>&1; then echo "::warning::gh copilot built-in unavailable with user token — rate-limit fallback will be unavailable." fi @@ -104,127 +94,18 @@ jobs: ;; esac - - name: Verify auth - env: - GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} - run: | - gh auth status - - name: Enumerate candidate PRs - id: list - env: - GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} run: | set -euo pipefail if [ -n "${PR_URL_OVERRIDE}" ]; then - echo "${PR_URL_OVERRIDE}" > prs.txt + printf '%s\n' "${PR_URL_OVERRIDE}" > prs.txt else bash scripts/list-prs.sh > prs.txt - TOTAL=$(wc -l < prs.txt | tr -d ' ') - if [ "$TOTAL" -gt 0 ]; then - echo "::notice::Candidate pool has $TOTAL PRs. Review loop will process until MAX_PRS actual reviews are completed (no-ops are skipped)." - fi fi - echo "count=$(wc -l < prs.txt | tr -d ' ')" >> "$GITHUB_OUTPUT" - echo "--- candidate PRs (will filter no-ops) ---" + count=$(grep -c . prs.txt || true) + echo "::notice::Candidate pool has $count PRs (no-ops will be skipped, MAX_PRS caps actual reviews)" + echo "--- candidate PRs ---" cat prs.txt || true - name: Review each PR (cascade) - if: steps.list.outputs.count != '0' - env: - GH_TOKEN: ${{ secrets.GH_PAT_WORKFLOWS }} - MAX_PRS: ${{ vars.MAX_PRS || '10' }} - CANDIDATE_LIMIT: ${{ vars.CANDIDATE_LIMIT || '100' }} - run: | - set -euo pipefail - actual=0 - skipped_noops=0 - failed=0 - engine_fallbacks=0 - processed=0 - session_aborted=0 - abort_pr="" - abort_reason="" - total_candidates=$(wc -l < prs.txt | tr -d ' ') - while IFS= read -r pr_url; do - [ -z "$pr_url" ] && continue - processed=$((processed + 1)) - - # Hard limit on candidates checked to avoid timeout - if [ "$processed" -gt "$CANDIDATE_LIMIT" ]; then - echo "::notice::Hit CANDIDATE_LIMIT=$CANDIDATE_LIMIT, stopping batch" - break - fi - - # Stop after MAX_PRS actual reviews (not counting no-ops) - if [ "$actual" -ge "$MAX_PRS" ]; then - echo "::notice::Reached MAX_PRS=$MAX_PRS actual reviews, stopping batch" - break - fi - - echo "::group::Reviewing $pr_url" - rc=0 - bash scripts/review-one-pr.sh "$pr_url" || rc=$? - - # Exit code 2 = engine rate-limited. - # If primary engine is Claude, switch to Copilot and retry this PR. - # All subsequent PRs in this batch will also use Copilot. - if [ "$rc" -eq 2 ] && [ "${REVIEW_ENGINE:-claude}" = "claude" ]; then - if ! gh extension list 2>/dev/null | grep -q copilot; then - echo "::warning::Copilot fallback engine unavailable (not installed) — skipping $pr_url and continuing batch" - failed=$((failed + 1)) - echo "::endgroup::" - continue - fi - echo "::warning::Claude rate limit hit — switching to Copilot engine for remaining PRs" - export REVIEW_ENGINE=copilot - engine_fallbacks=$((engine_fallbacks + 1)) - rc=0 - bash scripts/review-one-pr.sh "$pr_url" || rc=$? - fi - - if [ "$rc" -eq 100 ]; then - # Exit code 100 = no-op (already reviewed), don't count toward budget - skipped_noops=$((skipped_noops + 1)) - echo "::notice::No-op (already reviewed)" - elif [ "$rc" -eq 0 ]; then - # Successful review - actual=$((actual + 1)) - echo "::notice::Review posted ($actual/$MAX_PRS)" - elif [ "$rc" -eq 2 ]; then - # Rate-limited on fallback engine too — no further fallback available. - # Session-fatal: subsequent PRs will hit the same wall. - failed=$((failed + 1)) - echo "::error::Rate limit hit on $REVIEW_ENGINE engine, no fallback available for $pr_url" - session_aborted=1 - abort_pr="$pr_url" - abort_reason="rate-limit on fallback engine" - else - # Other failure — session-fatal. A systemic problem (model degraded, - # prompt regression, malformed verdicts) shouldn't burn the queue. - failed=$((failed + 1)) - echo "::error::Review failed for $pr_url (exit code $rc)" - session_aborted=1 - abort_pr="$pr_url" - abort_reason="exit code $rc" - fi - echo "::endgroup::" - if [ "$session_aborted" -eq 1 ]; then - break - fi - done < prs.txt - remaining=$((total_candidates - processed)) - if [ "$session_aborted" -eq 1 ]; then - echo "::error::Session aborted early after failure on $abort_pr ($abort_reason). Skipped $remaining remaining candidate(s); will retry on next scheduled run." - fi - if [ "$engine_fallbacks" -gt 0 ]; then - summary="Summary: $actual reviews posted, $skipped_noops no-ops skipped, $failed failures, $engine_fallbacks engine fallback(s) to copilot (processed $processed/$total_candidates candidates)" - else - summary="Summary: $actual reviews posted, $skipped_noops no-ops skipped, $failed failures (processed $processed/$total_candidates candidates)" - fi - if [ "$session_aborted" -eq 1 ]; then - echo "$summary [SESSION ABORTED EARLY]" - exit 1 - else - echo "$summary" - fi + run: bash scripts/review-batch.sh diff --git a/scripts/review-batch.sh b/scripts/review-batch.sh new file mode 100755 index 000000000..a65cd3b0c --- /dev/null +++ b/scripts/review-batch.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# Run the cascading review against every PR listed in $PRS_FILE (default +# prs.txt). One PR per line. Empty input is a no-op. +# +# Env (passed through from the workflow): +# PRS_FILE — path to candidate list (default: prs.txt) +# MAX_PRS — stop after this many actual reviews (no-ops don't count) +# CANDIDATE_LIMIT — hard cap on candidates inspected (timeout backstop) +# REVIEW_ENGINE — primary engine (claude|copilot); may be flipped on +# rate-limit fallback +# GH_TOKEN — workflow auth (set at job level) +# +# Exit: +# 0 — finished cleanly (zero or more reviews posted) +# 1 — session aborted early (a non-skip failure on some PR; remaining +# candidates are deferred to the next scheduled run) + +set -euo pipefail + +PRS_FILE="${PRS_FILE:-prs.txt}" +MAX_PRS="${MAX_PRS:-10}" +CANDIDATE_LIMIT="${CANDIDATE_LIMIT:-100}" + +if [ ! -s "$PRS_FILE" ]; then + echo "::notice::No candidate PRs to review." + exit 0 +fi + +actual=0 +skipped_noops=0 +failed=0 +engine_fallbacks=0 +processed=0 +session_aborted=0 +abort_pr="" +abort_reason="" +total_candidates=$(grep -c . "$PRS_FILE" || true) + +while IFS= read -r pr_url; do + [ -z "$pr_url" ] && continue + processed=$((processed + 1)) + + if [ "$processed" -gt "$CANDIDATE_LIMIT" ]; then + echo "::notice::Hit CANDIDATE_LIMIT=$CANDIDATE_LIMIT, stopping batch" + break + fi + if [ "$actual" -ge "$MAX_PRS" ]; then + echo "::notice::Reached MAX_PRS=$MAX_PRS actual reviews, stopping batch" + break + fi + + echo "::group::Reviewing $pr_url" + rc=0 + bash scripts/review-one-pr.sh "$pr_url" || rc=$? + + # Exit code 2 = engine rate-limited. Switch from claude to copilot once, + # then retry this PR. All later PRs in the batch use copilot too. + if [ "$rc" -eq 2 ] && [ "${REVIEW_ENGINE:-claude}" = "claude" ]; then + if ! gh extension list 2>/dev/null | grep -q copilot; then + echo "::warning::Copilot fallback engine unavailable (not installed) — skipping $pr_url and continuing batch" + failed=$((failed + 1)) + echo "::endgroup::" + continue + fi + echo "::warning::Claude rate limit hit — switching to Copilot engine for remaining PRs" + export REVIEW_ENGINE=copilot + engine_fallbacks=$((engine_fallbacks + 1)) + rc=0 + bash scripts/review-one-pr.sh "$pr_url" || rc=$? + fi + + case "$rc" in + 0) + actual=$((actual + 1)) + echo "::notice::Review posted ($actual/$MAX_PRS)" + ;; + 100) + skipped_noops=$((skipped_noops + 1)) + echo "::notice::No-op (already reviewed)" + ;; + 2) + failed=$((failed + 1)) + echo "::error::Rate limit hit on $REVIEW_ENGINE engine, no fallback available for $pr_url" + session_aborted=1 + abort_pr="$pr_url" + abort_reason="rate-limit on fallback engine" + ;; + *) + # Other failure — session-fatal. A systemic problem (model degraded, + # prompt regression, malformed verdicts) shouldn't burn the queue. + failed=$((failed + 1)) + echo "::error::Review failed for $pr_url (exit code $rc)" + session_aborted=1 + abort_pr="$pr_url" + abort_reason="exit code $rc" + ;; + esac + + echo "::endgroup::" + [ "$session_aborted" -eq 1 ] && break +done < "$PRS_FILE" + +remaining=$((total_candidates - processed)) +summary="Summary: $actual reviews posted, $skipped_noops no-ops skipped, $failed failures" +[ "$engine_fallbacks" -gt 0 ] && summary="$summary, $engine_fallbacks engine fallback(s) to copilot" +summary="$summary (processed $processed/$total_candidates candidates)" + +if [ "$session_aborted" -eq 1 ]; then + echo "::error::Session aborted early after failure on $abort_pr ($abort_reason). Skipped $remaining remaining candidate(s); will retry on next scheduled run." + echo "$summary [SESSION ABORTED EARLY]" + exit 1 +fi + +echo "$summary" From 285e8560f6f48f80fc8e6b1d8854b59093b07dd0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 10 May 2026 18:17:18 +0000 Subject: [PATCH 5/5] perf(pr-review): cache claude-code CLI install across runs `npm install -g @anthropic-ai/claude-code` ran on every workflow start (~30s). Switch to a per-user npm prefix (~/.npm-global) and cache that directory via actions/cache, keyed on CLAUDE_CODE_VERSION + runner OS. A `command -v claude` guard makes the install a no-op on cache hit, so the only cost on subsequent runs is the cache restore. CLAUDE_CODE_VERSION defaults to 'latest' (cache persists until manually flushed); set the repo variable to pin a specific version for fully reproducible caching. https://claude.ai/code/session_01EacTxiHSUhR6kxppXpmxig --- .github/workflows/pr-review.yml | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 056f154c2..827bdc32e 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -66,6 +66,10 @@ jobs: MAX_REVIEW_CYCLES: ${{ vars.MAX_REVIEW_CYCLES || '3' }} MAX_PRS: ${{ vars.MAX_PRS || '10' }} CANDIDATE_LIMIT: ${{ vars.CANDIDATE_LIMIT || '100' }} + # Pin via vars.CLAUDE_CODE_VERSION for fully reproducible caching; default + # 'latest' caches an install indefinitely under that key — flush via the + # Actions cache UI or bump the variable to pick up new releases. + CLAUDE_CODE_VERSION: ${{ vars.CLAUDE_CODE_VERSION || 'latest' }} # Mention-triggered reviews always force; workflow_dispatch respects the input. FORCE_REVIEW: ${{ github.event_name == 'repository_dispatch' && 'true' || inputs.force_review || 'false' }} @@ -73,20 +77,43 @@ jobs: - name: Checkout agent repo uses: actions/checkout@v5 + - name: Cache claude-code CLI + uses: actions/cache@v4 + with: + path: ~/.npm-global + key: claude-code-${{ env.CLAUDE_CODE_VERSION }}-${{ runner.os }} + - name: Install review engine CLIs run: | set -euo pipefail gh auth status + + # Use a per-user prefix so actions/cache (above) can persist the + # install across runs without needing root. Add the bin dir to PATH + # for both this step and every subsequent step in the job. + mkdir -p "$HOME/.npm-global" + npm config set prefix "$HOME/.npm-global" + echo "$HOME/.npm-global/bin" >> "$GITHUB_PATH" + export PATH="$HOME/.npm-global/bin:$PATH" + + install_claude() { + if command -v claude >/dev/null 2>&1; then + echo "claude-code cache hit: $(claude --version)" + return 0 + fi + npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}" + } + case "$REVIEW_ENGINE" in claude) - npm install -g @anthropic-ai/claude-code + install_claude if ! env GH_TOKEN="$COPILOT_GITHUB_TOKEN" gh copilot --version > /dev/null 2>&1; then echo "::warning::gh copilot built-in unavailable with user token — rate-limit fallback will be unavailable." fi ;; copilot) gh extension install github/gh-copilot - npm install -g @anthropic-ai/claude-code || true + install_claude || true ;; *) echo "::error::Unknown REVIEW_ENGINE=$REVIEW_ENGINE"