From aa1a0b9ec615e197b6dbf8821368028368c77210 Mon Sep 17 00:00:00 2001 From: DJ Date: Fri, 10 Apr 2026 04:52:07 -0700 Subject: [PATCH 1/3] Add review mode optimization: small-PR and incremental fast paths PRs are now routed to the cheapest review path: - Small PRs (<10 lines): single Opus call (1 invocation vs 4) - Re-reviews (prior marker at different SHA): single Opus call with prior review context, focused on what changed since last review - First review of non-small PRs: full 3-member council (unchanged) New files: - prompts/single-review.md: combined council+synth prompt for single reviewer mode, handles both small and incremental paths New config: - SMALL_PR_THRESHOLD repo variable (default 10) Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/pr-review.yml | 3 + AGENT.md | 14 +++- prompts/single-review.md | 135 ++++++++++++++++++++++++++++++++ scripts/review-one-pr.sh | 48 +++++++++++- 4 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 prompts/single-review.md diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 5bdcaeecb..dd96c1f1d 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -45,6 +45,9 @@ jobs: CLAUDE_ORGS: ${{ vars.CLAUDE_ORGS || '' }} # Max review cycles before stopping @claude delegation and escalating to human. MAX_REVIEW_CYCLES: ${{ vars.MAX_REVIEW_CYCLES || '3' }} + # PRs with fewer total changed lines than this use a single Opus call + # instead of the full 3-member council. Default: 10 lines. + SMALL_PR_THRESHOLD: ${{ vars.SMALL_PR_THRESHOLD || '10' }} steps: - name: Checkout agent repo diff --git a/AGENT.md b/AGENT.md index 5f46f74c0..09fe3915b 100644 --- a/AGENT.md +++ b/AGENT.md @@ -45,7 +45,16 @@ quality gates, and escalates high-risk or gated PRs for human review. delegation without resolution, the agent stops delegating and escalates to human to prevent infinite loops. -5. **Idempotency + iterative review cycles** — every posted review starts with +5. **Review mode optimization** — the agent picks the cheapest review path: + - **Small PR** (<10 lines, configurable via `SMALL_PR_THRESHOLD`): single + Opus call instead of 3-member council + synth (1 invocation vs 4). + - **Incremental re-review** (prior marker exists at a different SHA): single + Opus call that reads the prior review and focuses on what changed. Checks + whether prior findings are resolved and flags any new issues. + - **Full council** (first review of a non-small PR): 3 parallel members + + synthesizer (the original path). + +6. **Idempotency + iterative review cycles** — every posted review starts with an HTML marker on line 1: ``` @@ -149,6 +158,9 @@ gh workflow run pr-review.yml --repo don-petry/self -f dry_run=false - **Max review cycles** — how many times the agent tags @claude before escalating to human (default 3): `gh variable set MAX_REVIEW_CYCLES --body 5 --repo don-petry/self` +- **Small PR threshold** — PRs with fewer lines than this skip the council + and use a single Opus call (default 10): + `gh variable set SMALL_PR_THRESHOLD --body 20 --repo don-petry/self` - **Max PRs per run** — defaults to 10 per cron tick to stay within the 60-min job timeout (~5 min per PR with 3 council members). Override: `gh variable set MAX_PRS --body 15 --repo don-petry/self` diff --git a/prompts/single-review.md b/prompts/single-review.md new file mode 100644 index 000000000..4c2e8d89f --- /dev/null +++ b/prompts/single-review.md @@ -0,0 +1,135 @@ +# Single-reviewer mode + +You are a combined PR-review agent acting on behalf of GitHub user `don-petry`. +You run inside a GitHub Action with `gh` CLI authenticated. You perform the work +of the full review council (security + correctness + maintainability) and +synthesizer in a single pass. + +This mode is used when the full 3-member council is overkill — either the PR +is small or this is a re-review after a prior council review. + +## Inputs (environment variables) + +- `$PR_URL` — the PR to review. +- `$PR_HEAD_SHA` — the head commit SHA. +- `$DRY_RUN` — `true` or `false`. +- `$CLAUDE_ENABLED` — `true` or `false` (repo org has Claude App). +- `$REVIEW_CYCLE` — integer, number of prior review cycles. +- `$MAX_REVIEW_CYCLES` — integer, max cycles before human escalation. +- `$REVIEW_MODE` — `small` or `incremental`. +- `$PRIOR_REVIEW_BODY` — (incremental mode only) the body of the most recent + prior review from the council, so you can see what was previously flagged. +- `$PRIOR_REVIEW_SHA` — (incremental mode only) the SHA that was previously + reviewed. + +## Hard scope + +You review **exactly one pull request**: `$PR_URL`. Nothing else. + +**FORBIDDEN** — do not run: +- `gh search prs`, `gh pr list`, `gh pr status`, or any enumeration command. +- Any action on any PR other than `$PR_URL`. + +## Context-gathering + +1. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` + - If `isDraft` → skip. Print `{"pr":"...","decision":"skip","reason":"draft"}` and exit. + - Verify `headRefOid == $PR_HEAD_SHA`. If not → skip with `"reason":"head-sha-changed"`. +2. `gh pr diff "$PR_URL"` — read the diff. + - **Incremental mode**: also get the diff since the prior review: + `gh api "repos///compare/$PRIOR_REVIEW_SHA...$PR_HEAD_SHA" --jq '.commits[].commit.message, .files[].filename'` + to understand what changed since last review. Focus your analysis on + what's new. +3. Fetch linked issues (same as shared.md). +4. Inspect `statusCheckRollup`. + +## Risk classification + +Use the same taxonomy as the full council (from shared.md): + +### HIGH (never auto-approve) +- Auth, secrets, credentials, crypto, tokens, `.env*` +- DB migrations, schema changes +- Security anti-patterns (SQL injection, eval, shell=True, etc.) +- CI security warnings (CodeQL, Semgrep, Snyk, etc.) +- Org/project standards violations (CONTRIBUTING.md, AGENTS.md, CODEOWNERS) +- GitHub Actions security smells + +### MEDIUM +- Non-trivial logic changes, new dependencies, cross-module refactors + +### LOW +- Docs-only, comments, typos, test-only, lockfile updates + +## Decision gates + +Approve only if ALL: +1. Risk is LOW or MEDIUM (never HIGH) +2. All CI checks green +3. Linked issue substantively addressed +4. No unresolved review threads +5. No unanswered human-reviewer questions +6. Well-structured PR + +Otherwise → escalate. + +### Incremental mode adjustments + +When `$REVIEW_MODE` is `incremental`, your job is to determine if the new +commits resolved the issues from the prior review. Read `$PRIOR_REVIEW_BODY` +carefully. For each finding in the prior review: +- If the new commits fix it → note as resolved +- If the new commits don't address it → carry it forward +- If the new commits introduce NEW issues → flag them + +If all prior findings are resolved AND no new issues → approve. + +## Actions (same as synthesizer) + +Compose a review body with the same template: + +``` + + +## Automated review — + +**Risk:** +**Reviewed commit:** `` +**Review mode:** (single reviewer) + +### Summary +<2-4 sentences> + +### Linked issue analysis + + +### Findings + + +### CI status + + +--- +_Reviewed automatically by the don-petry PR-review agent (single-reviewer mode: opus 4.6). Reply with `@don-petry` if you need a human._ +``` + +Then act: + +- If `$DRY_RUN` is `true`: print `--- WOULD POST ---`, the body, and what + actions you would take. Exit. +- If approving: + 1. `gh pr review "$PR_URL" --approve --body "$BODY"` + 2. Rebase if `mergeStateStatus` is `BEHIND`: + `gh api -X PUT "repos///pulls//update-branch" -f expected_head_sha="$PR_HEAD_SHA"` (swallow errors) + 3. Enable auto-merge: `gh pr merge "$PR_URL" --auto --squash` (swallow errors) + 4. Remove `needs-human-review` label if present (swallow errors) +- If escalating: + - If `$CLAUDE_ENABLED` is `true` AND `$REVIEW_CYCLE` < `$MAX_REVIEW_CYCLES` + AND risk is NOT `HIGH`: + Post a fix-request issue comment (see synthesize.md step 9a for template). + - Otherwise: add `needs-human-review` label, re-request don-petry as reviewer. + +After acting, print: +```json +{"pr":"","sha":"","risk":"","decision":"","mode":"","delegated_to":"claude|human|none","posted":true|false} +``` diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index e00d45dee..3f123ef55 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -77,7 +77,53 @@ fi export CLAUDE_ENABLED echo " claude delegation: $CLAUDE_ENABLED (org: $PR_ORG)" -# 3. Run council in parallel +# 3. Determine review mode based on PR size and prior review history. +# - "small" : PR has < SMALL_PR_THRESHOLD lines changed → single Opus call +# - "incremental" : prior review exists at different SHA → single Opus call with prior context +# - "full" : first review of a non-small PR → 3-member council + synthesizer +SMALL_PR_THRESHOLD="${SMALL_PR_THRESHOLD:-10}" +PR_SIZE=$(gh pr view "$PR_URL" --json additions,deletions --jq '.additions + .deletions') +export PR_SIZE + +if [ "$PR_SIZE" -lt "$SMALL_PR_THRESHOLD" ]; then + REVIEW_MODE="small" +elif [ -n "${EXISTING_MARKER_SHA:-}" ]; then + REVIEW_MODE="incremental" +else + REVIEW_MODE="full" +fi +export REVIEW_MODE +echo " mode: $REVIEW_MODE (${PR_SIZE} lines, threshold: $SMALL_PR_THRESHOLD)" + +# For incremental mode, extract the prior review body so the single reviewer +# can see what was previously flagged and check if issues are resolved. +if [ "$REVIEW_MODE" = "incremental" ]; then + PRIOR_REVIEW_SHA="$EXISTING_MARKER_SHA" + export PRIOR_REVIEW_SHA + # Extract the full body of the review/comment containing our marker for the prior SHA. + PRIOR_REVIEW_BODY=$( + gh pr view "$PR_URL" --json reviews,comments \ + --jq "((.reviews // []) + (.comments // [])) | .[].body | select(. != null) | select(test(\"sha=$PRIOR_REVIEW_SHA\"))" 2>/dev/null \ + | head -1 || true + ) + export PRIOR_REVIEW_BODY + echo " prior review SHA: $PRIOR_REVIEW_SHA (body: ${#PRIOR_REVIEW_BODY} chars)" +fi + +# --- Single-reviewer mode (small PR or incremental re-review) --- +if [ "$REVIEW_MODE" != "full" ]; then + echo " [single] opus (mode=$REVIEW_MODE)" + claude \ + --print \ + --model claude-opus-4-6 \ + --permission-mode acceptEdits \ + --allowed-tools "Bash,Read,Grep,Glob" \ + < prompts/single-review.md + echo " [done] $PR_URL" + exit 0 +fi + +# --- Full council mode (first review of a non-small PR) --- mkdir -p /tmp/council rm -f /tmp/council/*.json /tmp/council/*.log From bab010f29438d21527bddf9c045733346b603b11 Mon Sep 17 00:00:00 2001 From: DJ Date: Fri, 10 Apr 2026 05:09:07 -0700 Subject: [PATCH 2/3] Replace parallel council with cascading review tiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Haiku triage → Sonnet deep review → Opus security audit, where each tier only fires if the previous one escalated. This replaces the 3-parallel-members + synthesizer architecture. Cost impact: - ~80% of PRs: Haiku + Opus confirm (2 calls, ~30s vs 4 calls, ~5 min) - ~15%: + Sonnet (3 calls, ~2.5 min) - ~5%: full cascade (4 calls, ~5.5 min) Haiku works by receiving pre-fetched context (no tool use needed), which sidesteps the agentic limitations found earlier. New files: - prompts/triage.md — Tier 1: Haiku fast classification - prompts/deep-review.md — Tier 2: Sonnet full review - prompts/security-audit.md — Tier 3: Opus security audit - prompts/cascade-action.md — posts review from any resolving tier Re-reviews (prior marker at different SHA) pass prior review context to the triage, enabling faster incremental checks. Co-Authored-By: Claude Opus 4.6 (1M context) --- AGENT.md | 59 ++++++----- prompts/cascade-action.md | 83 ++++++++++++++++ prompts/deep-review.md | 88 +++++++++++++++++ prompts/security-audit.md | 81 +++++++++++++++ prompts/triage.md | 61 ++++++++++++ scripts/review-one-pr.sh | 203 ++++++++++++++++++++++---------------- 6 files changed, 457 insertions(+), 118 deletions(-) create mode 100644 prompts/cascade-action.md create mode 100644 prompts/deep-review.md create mode 100644 prompts/security-audit.md create mode 100644 prompts/triage.md diff --git a/AGENT.md b/AGENT.md index 09fe3915b..97e0ec582 100644 --- a/AGENT.md +++ b/AGENT.md @@ -11,24 +11,31 @@ quality gates, and escalates high-risk or gated PRs for human review. 2. **Enumerate** — `scripts/list-prs.sh` queries GitHub for open PRs where `@me` is the author OR a requested reviewer, across every repo the PAT can see. Output: one URL per line. -3. **Per-PR review** — for each PR, `scripts/review-one-pr.sh` orchestrates a - council of three Claude models, each with a focused lens, then a synthesizer - that posts a single combined review: - - | Lens | Model | What it looks for | - |---|---|---| - | Security | Opus 4.6 | auth, secrets, injection, supply chain, GH Actions security smells | - | Correctness | Sonnet 4.6 | linked-issue alignment, logic bugs, edge cases, test coverage, CI | - | Maintainability | Sonnet 4.6 | org standards, conventions, clarity, dependency hygiene | - | Synthesis | Sonnet 4.6 | combines verdicts, takes max risk, dedupes findings, posts to GitHub | - - The three council members run in parallel, each writing a JSON verdict to - `/tmp/council/.json`. None of them touch GitHub. The synthesizer then - reads all three, takes `max(risk)` and `escalate if any escalates`, dedupes - findings, and posts **one** PR review: - - If approved: `gh pr review --approve` with the combined summary. - - If escalated: `gh pr review --comment`, re-requests don-petry as a - reviewer, and adds the `needs-human-review` label. +3. **Per-PR review** — `scripts/review-one-pr.sh` runs a cascading review + where each tier only fires if the previous one flagged concerns: + + ``` + Tier 1: Haiku triage (~15s, no tools, pre-fetched context) + └─ clean? → single Opus confirmation → approve + auto-merge + └─ concerns? ↓ + Tier 2: Sonnet deep review (~2 min, full agentic) + └─ clean? → approve + auto-merge + └─ HIGH risk? ↓ + Tier 3: Opus security audit (~3 min, full agentic) + └─ final decision → approve or escalate + ``` + + | Tier | Model | Role | When | + |---|---|---|---| + | Triage | Haiku 4.5 | Fast risk classification, no tools | Every PR | + | Deep review | Sonnet 4.6 | Full review (security+correctness+maintainability) | Only if triage escalates | + | Security audit | Opus 4.6 | Paranoid security-focused review | Only if Sonnet flags HIGH risk | + | Action | Sonnet 4.6 | Posts review, handles delegation/merge | After resolving tier | + + **Cost profile:** + - ~80% of PRs: Haiku triage + Opus confirm (2 calls, ~30s) + - ~15% of PRs: + Sonnet deep review (3 calls, ~2.5 min) + - ~5% of PRs: + Opus audit (4 calls, ~5.5 min) 4. **Post-review actions** — after the review is posted, the synthesizer takes additional actions depending on the decision: @@ -45,16 +52,7 @@ quality gates, and escalates high-risk or gated PRs for human review. delegation without resolution, the agent stops delegating and escalates to human to prevent infinite loops. -5. **Review mode optimization** — the agent picks the cheapest review path: - - **Small PR** (<10 lines, configurable via `SMALL_PR_THRESHOLD`): single - Opus call instead of 3-member council + synth (1 invocation vs 4). - - **Incremental re-review** (prior marker exists at a different SHA): single - Opus call that reads the prior review and focuses on what changed. Checks - whether prior findings are resolved and flags any new issues. - - **Full council** (first review of a non-small PR): 3 parallel members + - synthesizer (the original path). - -6. **Idempotency + iterative review cycles** — every posted review starts with +5. **Idempotency + iterative review cycles** — every posted review starts with an HTML marker on line 1: ``` @@ -158,9 +156,8 @@ gh workflow run pr-review.yml --repo don-petry/self -f dry_run=false - **Max review cycles** — how many times the agent tags @claude before escalating to human (default 3): `gh variable set MAX_REVIEW_CYCLES --body 5 --repo don-petry/self` -- **Small PR threshold** — PRs with fewer lines than this skip the council - and use a single Opus call (default 10): - `gh variable set SMALL_PR_THRESHOLD --body 20 --repo don-petry/self` +- **Models** — change model IDs in `scripts/review-one-pr.sh`. The cascade + tiers map to: triage=haiku, deep=sonnet, audit=opus, action=sonnet. - **Max PRs per run** — defaults to 10 per cron tick to stay within the 60-min job timeout (~5 min per PR with 3 council members). Override: `gh variable set MAX_PRS --body 15 --repo don-petry/self` diff --git a/prompts/cascade-action.md b/prompts/cascade-action.md new file mode 100644 index 000000000..3e447f157 --- /dev/null +++ b/prompts/cascade-action.md @@ -0,0 +1,83 @@ +# Cascade action — post review based on tier result + +You are the final action step of the cascading PR review. A previous tier +(Sonnet or Opus) has produced a verdict in `$FINAL_RESULT`. Your job is to +read that verdict and post the review to GitHub. + +## Inputs (environment variables) + +- `$PR_URL` — the PR to act on. +- `$PR_HEAD_SHA` — the commit SHA that was reviewed. +- `$DRY_RUN` — `true` or `false`. +- `$CLAUDE_ENABLED` — `true` or `false`. +- `$REVIEW_CYCLE` — integer. +- `$MAX_REVIEW_CYCLES` — integer. +- `$FINAL_RESULT` — path to the JSON verdict from the resolving tier. +- `$FINAL_TIER` — `sonnet` or `opus` — which model made the final call. +- `$TRIAGE_RESULT` — JSON from the Haiku triage (for context). + +## Steps + +1. Read the JSON at `$FINAL_RESULT`. Extract `decision`, `risk`, `findings`, + `summary`, and `reason_codes`. +2. Fetch `mergeStateStatus` from the PR: + `gh pr view "$PR_URL" --json mergeStateStatus --jq '.mergeStateStatus'` +3. **Idempotency check**: look for our marker at `$PR_HEAD_SHA` in existing + reviews/comments (same as synthesize.md step 5). If found → noop. +4. Compose the review body using this template: + +``` + + +## Automated review — + +**Risk:** +**Reviewed commit:** `` +**Cascade:** triage(haiku) → + +### Summary + + +### Findings + + +### CI status + + +--- +_Reviewed by the don-petry PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 → audit: opus 4.6). Reply with `@don-petry` if you need a human._ +``` + +5. **Act** (same logic as synthesize.md): + - If `$DRY_RUN` is `true`: print `--- WOULD POST ---`, the body, and + planned actions. Exit. + - If `decision` is `approve`: + 1. `gh pr review "$PR_URL" --approve --body "$BODY"` + 2. Rebase if `mergeStateStatus` is `BEHIND`: + `gh api -X PUT "repos///pulls//update-branch" -f expected_head_sha="$PR_HEAD_SHA"` (swallow errors) + 3. Auto-merge: `gh pr merge "$PR_URL" --auto --squash` (swallow errors) + 4. Remove `needs-human-review` label if present (swallow errors) + - If `decision` is `escalate`: + - If `$CLAUDE_ENABLED` is `true` AND `$REVIEW_CYCLE` < `$MAX_REVIEW_CYCLES` + AND `risk` is NOT `HIGH`: + Post fix-request issue comment (NOT a review): + ``` + ## Review council — fix requested (cycle /) + + The automated review identified the following issues. Please address each one: + + ### Findings to fix + + - **[]** `:` — + + ### Additional tasks + 1. Resolve all unresolved review thread comments from other reviewers + 2. Ensure all CI checks pass after your changes + 3. Rebase on `` if the branch is behind + 4. Do NOT modify files unrelated to the findings above + + _The review cascade will automatically re-review after new commits are pushed._ + ``` + - Otherwise: add `needs-human-review` label, re-request don-petry. +6. Print status JSON: + `{"pr":"","sha":"","risk":"","decision":"","tier":"","delegated_to":"claude|human|none","posted":true|false}` diff --git a/prompts/deep-review.md b/prompts/deep-review.md new file mode 100644 index 000000000..cf18aec17 --- /dev/null +++ b/prompts/deep-review.md @@ -0,0 +1,88 @@ +# Tier 2: Deep review (Sonnet) + +You are the second tier of a cascading PR review. The fast triage (Haiku) +flagged this PR for deeper analysis. Your job is to do a thorough review +covering security, correctness, AND maintainability — then decide whether +to approve or escalate further to the security auditor (Opus). + +## Inputs (environment variables) + +- `$PR_URL` — the PR to review. +- `$PR_HEAD_SHA` — the head commit SHA. +- `$DRY_RUN` — `true` or `false`. +- `$CLAUDE_ENABLED` — `true` or `false`. +- `$REVIEW_CYCLE` — integer. +- `$MAX_REVIEW_CYCLES` — integer. +- `$TRIAGE_RESULT` — JSON output from the Haiku triage, including its + `signals` array explaining why it escalated. +- `$PRIOR_REVIEW_BODY` — prior review body if this is a re-review (empty if first). +- `$PRIOR_REVIEW_SHA` — prior SHA if re-review. + +## Scope + +You review **exactly one PR**: `$PR_URL`. You have `gh` CLI available. + +**FORBIDDEN**: `gh search prs`, `gh pr list`, `gh pr status`, or any +enumeration. No actions on other PRs. + +## Steps + +1. Read `$TRIAGE_RESULT` to understand why triage escalated — focus your + review on those signals. +2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` +3. `gh pr diff "$PR_URL"` — read the diff. +4. Fetch linked issues if any. +5. Check `statusCheckRollup` for CI status. + +## Risk classification + +Same taxonomy as shared.md: + +### HIGH → escalate to Opus (Tier 3) +- Auth/secrets/credentials/crypto/tokens/`.env*` +- DB migrations/schema changes +- Security anti-patterns (injection, eval, shell=True, hardcoded secrets, etc.) +- CI security scanner warnings +- Org/project standards violations +- GitHub Actions security smells + +### MEDIUM → you can approve if all gates pass +- Non-trivial logic changes, new deps, cross-module refactors + +### LOW → you can approve if all gates pass +- Docs, comments, typos, tests-only, lockfile updates + +## Decision + +- If risk is **HIGH** → write your findings to `$OUTPUT_FILE` and let Opus + handle the final decision. +- If risk is **LOW or MEDIUM** AND all gates pass (CI green, issue addressed, + no unresolved threads, well-structured) → approve. +- If risk is LOW/MEDIUM but a gate fails → escalate (your own findings are + sufficient, no need for Opus). + +## Output + +Write a JSON object to `$OUTPUT_FILE`: + +```json +{ + "tier": "sonnet", + "escalate_to_opus": true|false, + "risk": "LOW|MEDIUM|HIGH", + "decision": "approve|escalate", + "reason_codes": ["..."], + "summary": "2-3 sentences", + "findings": [ + { + "severity": "info|minor|major|critical", + "category": "...", + "message": "...", + "file": "path or null", + "line": "number or null" + } + ] +} +``` + +Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`. diff --git a/prompts/security-audit.md b/prompts/security-audit.md new file mode 100644 index 000000000..279030350 --- /dev/null +++ b/prompts/security-audit.md @@ -0,0 +1,81 @@ +# Tier 3: Security audit (Opus) + +You are the final tier of a cascading PR review — the security auditor. +Both the fast triage (Haiku) and the deep reviewer (Sonnet) flagged this PR +as needing expert security analysis. You are the most thorough and expensive +reviewer, called only for PRs with real concerns. + +## Inputs (environment variables) + +- `$PR_URL` — the PR to review. +- `$PR_HEAD_SHA` — the head commit SHA. +- `$DRY_RUN` — `true` or `false`. +- `$CLAUDE_ENABLED` — `true` or `false`. +- `$REVIEW_CYCLE` — integer. +- `$MAX_REVIEW_CYCLES` — integer. +- `$TRIAGE_RESULT` — JSON from the Haiku triage. +- `$SONNET_RESULT` — path to the Sonnet deep review JSON file. +- `$PRIOR_REVIEW_BODY` — prior review body if re-review. +- `$PRIOR_REVIEW_SHA` — prior SHA if re-review. + +## Scope + +**Exactly one PR**: `$PR_URL`. You have `gh` CLI. + +**FORBIDDEN**: enumeration commands, actions on other PRs. + +## Steps + +1. Read `$TRIAGE_RESULT` (Haiku's signals) and the Sonnet verdict at + `$SONNET_RESULT` (its findings, risk, and reasoning). +2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` +3. `gh pr diff "$PR_URL"` — read the diff. Focus on the areas Sonnet flagged. +4. Fetch linked issues if any. +5. Read any CONTRIBUTING.md, AGENTS.md, CODEOWNERS in the repo to check + standards compliance (fetch via `gh api`). + +## Your focus + +You are the **paranoid** reviewer. Your focus areas, in order: +1. AuthN/AuthZ, secrets, credential handling +2. Input validation, injection attacks (SQL, command, XSS, SSRF) +3. Crypto (weak algorithms, custom crypto, hardcoded keys) +4. Supply chain (dependency typosquats, unpinned actions, lockfile drift) +5. GitHub Actions security (pull_request_target, secret exposure, expression injection) +6. Data exposure (PII in logs, missing access controls, CORS wildcards) +7. The specific signals Haiku and Sonnet raised + +When uncertain between risk levels, round UP. You are the last line of defense. + +## Decision + +You make the final call: +- `approve` — only if you are confident the PR is safe and all gates pass. +- `escalate` — if any security concern remains, or any gate fails. + +## Output + +Write a JSON object to `$OUTPUT_FILE`: + +```json +{ + "tier": "opus", + "risk": "LOW|MEDIUM|HIGH", + "decision": "approve|escalate", + "reason_codes": ["..."], + "summary": "2-3 sentences", + "findings": [ + { + "severity": "info|minor|major|critical", + "category": "...", + "message": "...", + "file": "path or null", + "line": "number or null" + } + ], + "sonnet_findings_confirmed": [""], + "sonnet_findings_dismissed": [""] +} +``` + +Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`. diff --git a/prompts/triage.md b/prompts/triage.md new file mode 100644 index 000000000..636f5e8e2 --- /dev/null +++ b/prompts/triage.md @@ -0,0 +1,61 @@ +# Tier 1: Triage (Haiku) + +You are a fast PR triage agent. Your ONLY job is to read the pre-fetched PR +context below and decide: does this PR need a deeper review, or is it safe +to approve? + +You have NO tools. All context is provided in the environment variables below. +Do not attempt to run commands. Just read and decide. + +## Inputs (environment variables, pre-fetched by the orchestrator) + +- `$PR_URL` — the PR URL. +- `$PR_HEAD_SHA` — the head commit SHA. +- `$PR_METADATA` — JSON from `gh pr view` (title, body, author, files, checks, + linked issues, reviews, comments, additions, deletions, etc.). +- `$PR_DIFF` — the full diff (may be truncated for very large PRs). +- `$DRY_RUN` — `true` or `false` (for your awareness, you never post anything). +- `$REVIEW_MODE` — `triage` (always, for you). +- `$PRIOR_REVIEW_BODY` — if this is a re-review, the body of the prior review. + Empty string if first review. + +## Your decision criteria + +Output `"escalate": false` (approve) if ALL of these are true: +1. The diff touches NONE of these high-risk areas: + - Authentication, authorization, secrets, credentials, crypto, tokens, `.env*` + - Database migrations or schema (`migrations/`, `schema.*`, `*.sql`, Prisma, Alembic) + - GitHub Actions workflows that handle secrets or use `pull_request_target` + - Files matching: `**/auth/**`, `**/*secret*`, `**/*credential*`, `**/*crypto*` +2. No CI checks are failing (look at `statusCheckRollup` in metadata). +3. No unresolved review threads requesting changes. +4. The diff does not contain obvious security anti-patterns: + - SQL string concatenation, `eval`/`exec` on dynamic input, `shell=True` + with user input, hardcoded secrets/passwords, disabled TLS verification, + broad `except:` swallowing, `dangerouslySetInnerHTML`, etc. +5. If there's a linked issue, the diff appears to address it (use your judgment). +6. The PR is well-structured (clear title, reasonable scope). +7. If `$PRIOR_REVIEW_BODY` is non-empty: the new commits appear to resolve + the findings from the prior review. + +Output `"escalate": true` if ANY of those checks fail. When in doubt, escalate. +False positives are fine (the next tier will sort it out). False negatives are not. + +## Output format + +Output EXACTLY one JSON object, nothing else. No markdown, no explanation, +no preamble. Just the JSON: + +```json +{ + "escalate": true|false, + "risk": "LOW|MEDIUM|HIGH", + "signals": ["", ""], + "summary": "" +} +``` + +If `escalate` is `false`, `signals` should be empty or contain only positive +notes. If `escalate` is `true`, `signals` must list every reason for escalation. + +IMPORTANT: Output ONLY the JSON object. No other text. diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index 3f123ef55..ed9df36bf 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -77,42 +77,65 @@ fi export CLAUDE_ENABLED echo " claude delegation: $CLAUDE_ENABLED (org: $PR_ORG)" -# 3. Determine review mode based on PR size and prior review history. -# - "small" : PR has < SMALL_PR_THRESHOLD lines changed → single Opus call -# - "incremental" : prior review exists at different SHA → single Opus call with prior context -# - "full" : first review of a non-small PR → 3-member council + synthesizer -SMALL_PR_THRESHOLD="${SMALL_PR_THRESHOLD:-10}" -PR_SIZE=$(gh pr view "$PR_URL" --json additions,deletions --jq '.additions + .deletions') -export PR_SIZE - -if [ "$PR_SIZE" -lt "$SMALL_PR_THRESHOLD" ]; then - REVIEW_MODE="small" -elif [ -n "${EXISTING_MARKER_SHA:-}" ]; then - REVIEW_MODE="incremental" -else - REVIEW_MODE="full" -fi -export REVIEW_MODE -echo " mode: $REVIEW_MODE (${PR_SIZE} lines, threshold: $SMALL_PR_THRESHOLD)" - -# For incremental mode, extract the prior review body so the single reviewer -# can see what was previously flagged and check if issues are resolved. -if [ "$REVIEW_MODE" = "incremental" ]; then +# For re-reviews, extract the prior review body for context. +PRIOR_REVIEW_BODY="" +PRIOR_REVIEW_SHA="" +if [ -n "${EXISTING_MARKER_SHA:-}" ]; then PRIOR_REVIEW_SHA="$EXISTING_MARKER_SHA" - export PRIOR_REVIEW_SHA - # Extract the full body of the review/comment containing our marker for the prior SHA. PRIOR_REVIEW_BODY=$( gh pr view "$PR_URL" --json reviews,comments \ --jq "((.reviews // []) + (.comments // [])) | .[].body | select(. != null) | select(test(\"sha=$PRIOR_REVIEW_SHA\"))" 2>/dev/null \ | head -1 || true ) - export PRIOR_REVIEW_BODY - echo " prior review SHA: $PRIOR_REVIEW_SHA (body: ${#PRIOR_REVIEW_BODY} chars)" + echo " prior review: SHA=$PRIOR_REVIEW_SHA body=${#PRIOR_REVIEW_BODY}chars" +fi +export PRIOR_REVIEW_BODY PRIOR_REVIEW_SHA + +# ========================================================================== +# 3. Cascading review: Haiku triage → Sonnet deep review → Opus security audit +# Each tier only fires if the previous one escalated. +# ========================================================================== + +mkdir -p /tmp/cascade + +# --- Tier 1: Haiku triage (no tools, pre-fetched context) --- +echo " [tier1] haiku triage" + +# Pre-fetch all context so Haiku needs no tool access. +PR_METADATA=$(gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files 2>/dev/null || echo '{}') +export PR_METADATA + +PR_DIFF=$(gh pr diff "$PR_URL" 2>/dev/null | head -3000 || echo "") +export PR_DIFF + +export REVIEW_MODE="triage" + +TRIAGE_LOG="/tmp/cascade/triage.log" +TRIAGE_RESULT=$( + claude \ + --print \ + --model claude-haiku-4-5-20251001 \ + --permission-mode plan \ + < prompts/triage.md 2>"$TRIAGE_LOG" +) || true + +# Validate triage output is JSON +if ! echo "$TRIAGE_RESULT" | jq empty 2>/dev/null; then + echo " [tier1] haiku returned non-JSON, escalating by default" + echo " haiku output: $TRIAGE_RESULT" + TRIAGE_RESULT='{"escalate":true,"risk":"MEDIUM","signals":["triage-output-invalid"],"summary":"triage failed, escalating"}' fi -# --- Single-reviewer mode (small PR or incremental re-review) --- -if [ "$REVIEW_MODE" != "full" ]; then - echo " [single] opus (mode=$REVIEW_MODE)" +export TRIAGE_RESULT +TRIAGE_ESCALATE=$(echo "$TRIAGE_RESULT" | jq -r '.escalate') +TRIAGE_RISK=$(echo "$TRIAGE_RESULT" | jq -r '.risk') +TRIAGE_SIGNALS=$(echo "$TRIAGE_RESULT" | jq -r '.signals | join(", ")') +echo " [tier1] escalate=$TRIAGE_ESCALATE risk=$TRIAGE_RISK signals=[$TRIAGE_SIGNALS]" + +# If triage says no concerns → use single-review prompt (Haiku approved, Opus confirms briefly) +if [ "$TRIAGE_ESCALATE" = "false" ]; then + echo " [approve] haiku cleared — running single Opus confirmation" + export REVIEW_MODE="triage-approved" claude \ --print \ --model claude-opus-4-6 \ @@ -123,76 +146,82 @@ if [ "$REVIEW_MODE" != "full" ]; then exit 0 fi -# --- Full council mode (first review of a non-small PR) --- -mkdir -p /tmp/council -rm -f /tmp/council/*.json /tmp/council/*.log - -run_member() { - local lens="$1" - local model="$2" - local prompt_path="$3" - local out="/tmp/council/${lens}.json" - local log="/tmp/council/${lens}.log" - local prompt_file="/tmp/council/${lens}.prompt" - - cat prompts/shared.md "$prompt_path" > "$prompt_file" - local prompt_bytes - prompt_bytes=$(wc -c < "$prompt_file") - echo " [start] $lens ($model) prompt=${prompt_bytes}b" - - LENS="$lens" \ - OUTPUT_FILE="$out" \ +# --- Tier 2: Sonnet deep review --- +echo " [tier2] sonnet deep review" +OUTPUT_FILE="/tmp/cascade/sonnet.json" +export OUTPUT_FILE +claude \ + --print \ + --model claude-sonnet-4-6 \ + --permission-mode acceptEdits \ + --allowed-tools "Bash,Read,Grep,Glob" \ + < prompts/deep-review.md \ + > /tmp/cascade/sonnet.log 2>&1 +SONNET_RC=$? + +if [ ! -s "$OUTPUT_FILE" ] || ! jq empty "$OUTPUT_FILE" 2>/dev/null; then + echo "::warning::sonnet did not produce valid JSON at $OUTPUT_FILE" + cat /tmp/cascade/sonnet.log || true + echo "::error::cascade failed at tier 2 for $PR_URL" + exit 1 +fi + +SONNET_ESCALATE=$(jq -r '.escalate_to_opus' "$OUTPUT_FILE") +SONNET_DECISION=$(jq -r '.decision' "$OUTPUT_FILE") +SONNET_RISK=$(jq -r '.risk' "$OUTPUT_FILE") +echo " [tier2] escalate_to_opus=$SONNET_ESCALATE decision=$SONNET_DECISION risk=$SONNET_RISK" + +# If Sonnet approves or escalates without needing Opus → go straight to action +if [ "$SONNET_ESCALATE" != "true" ]; then + echo " [action] sonnet resolved — posting review" + FINAL_RESULT="$OUTPUT_FILE" + export FINAL_RESULT + export FINAL_TIER="sonnet" claude \ --print \ - --model "$model" \ + --model claude-sonnet-4-6 \ --permission-mode acceptEdits \ --allowed-tools "Bash,Read,Grep,Glob" \ - < "$prompt_file" \ - > "$log" 2>&1 - local rc=$? - echo " [done] $lens (rc=$rc)" - return $rc -} - -run_member security claude-opus-4-6 prompts/council/security.md & -PID_SEC=$! -run_member correctness claude-sonnet-4-6 prompts/council/correctness.md & -PID_COR=$! -run_member maintainability claude-sonnet-4-6 prompts/council/maintainability.md & -PID_MAI=$! - -FAILED=0 -wait "$PID_SEC" || { echo "::warning::security council member failed"; FAILED=1; } -wait "$PID_COR" || { echo "::warning::correctness council member failed"; FAILED=1; } -wait "$PID_MAI" || { echo "::warning::maintainability council member failed"; FAILED=1; } - -# Verify each member produced parseable JSON -for lens in security correctness maintainability; do - f="/tmp/council/${lens}.json" - if [ ! -s "$f" ]; then - echo "::warning::$lens did not produce $f" - echo "--- $lens log ---" - cat "/tmp/council/${lens}.log" || true - FAILED=1 - elif ! jq empty "$f" 2>/dev/null; then - echo "::warning::$lens produced invalid JSON in $f" - cat "$f" - FAILED=1 - fi -done - -if [ "$FAILED" -ne 0 ]; then - echo "::error::council had failures, skipping synthesis for $PR_URL" + < prompts/cascade-action.md + echo " [done] $PR_URL" + exit 0 +fi + +# --- Tier 3: Opus security audit --- +echo " [tier3] opus security audit" +SONNET_RESULT="$OUTPUT_FILE" +export SONNET_RESULT +OUTPUT_FILE="/tmp/cascade/opus.json" +export OUTPUT_FILE +claude \ + --print \ + --model claude-opus-4-6 \ + --permission-mode acceptEdits \ + --allowed-tools "Bash,Read,Grep,Glob" \ + < prompts/security-audit.md \ + > /tmp/cascade/opus.log 2>&1 + +if [ ! -s "$OUTPUT_FILE" ] || ! jq empty "$OUTPUT_FILE" 2>/dev/null; then + echo "::warning::opus did not produce valid JSON at $OUTPUT_FILE" + cat /tmp/cascade/opus.log || true + echo "::error::cascade failed at tier 3 for $PR_URL" exit 1 fi -# 4. Synthesize and (maybe) post -echo " [synth]" +OPUS_DECISION=$(jq -r '.decision' "$OUTPUT_FILE") +OPUS_RISK=$(jq -r '.risk' "$OUTPUT_FILE") +echo " [tier3] decision=$OPUS_DECISION risk=$OPUS_RISK" + +# Opus makes the final call — post the review +echo " [action] opus resolved — posting review" +FINAL_RESULT="$OUTPUT_FILE" +export FINAL_RESULT +export FINAL_TIER="opus" claude \ --print \ --model claude-sonnet-4-6 \ --permission-mode acceptEdits \ --allowed-tools "Bash,Read,Grep,Glob" \ - < prompts/synthesize.md + < prompts/cascade-action.md echo " [done] $PR_URL" From 1acc010ea86c75c732b0c6a15d87a13526e4ea6b Mon Sep 17 00:00:00 2001 From: DJ Date: Fri, 10 Apr 2026 09:58:35 -0700 Subject: [PATCH 3/3] Fix review council findings: 4 major + 3 minor issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Major fixes: - Replace invalid 'repository' gh field with 'headRepository'/'headRepositoryOwner' across script and all prompts - Add 'triage-approved' as valid REVIEW_MODE in single-review.md with dedicated mode description - Fix PRIOR_REVIEW_BODY selector: head -1 → tail -1 to match most-recent SHA - Update dangling 'synthesize.md step 9a' reference to cascade-action.md Minor fixes: - Validate PRIOR_REVIEW_BODY contains marker before exporting (anti-injection) - Write prior review body to temp file to avoid E2BIG on large reviews - Remove dead SONNET_RC=$? variable; add || true for set -e safety - Delete orphaned prompts/council/ files (no longer referenced by cascade) - Fix incremental-mode compare command: document how to derive owner/repo from headRepository metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- prompts/council/correctness.md | 37 ------------------------- prompts/council/maintainability.md | 44 ------------------------------ prompts/council/security.md | 39 -------------------------- prompts/deep-review.md | 2 +- prompts/security-audit.md | 2 +- prompts/single-review.md | 28 +++++++++++++------ scripts/review-one-pr.sh | 24 ++++++++++++---- 7 files changed, 40 insertions(+), 136 deletions(-) delete mode 100644 prompts/council/correctness.md delete mode 100644 prompts/council/maintainability.md delete mode 100644 prompts/council/security.md diff --git a/prompts/council/correctness.md b/prompts/council/correctness.md deleted file mode 100644 index 2029c18b9..000000000 --- a/prompts/council/correctness.md +++ /dev/null @@ -1,37 +0,0 @@ -# Council member: CORRECTNESS lens - -Your `$LENS` is `correctness`. You are the correctness-focused reviewer. -Your job is to determine whether the code does what it claims to do. - -## Focus areas (in priority order) - -1. **Linked issue alignment** — read every linked issue in detail. List the - acceptance criteria you can identify. For each criterion, state explicitly - whether the diff addresses it (and how) or not. If there is no linked issue - and the PR isn't trivial (docs/typo), note it as a concern but not auto-fail. -2. **Logic correctness** — off-by-one, null/undefined handling, error paths, - edge cases, race conditions, concurrency bugs, ordering assumptions, - numeric overflow, time-zone handling, locale handling. -3. **API contract** — does the change break callers? Is backward compatibility - preserved where it should be? Are deprecations marked? -4. **Test coverage of the change** — are new code paths tested? Do existing - tests still cover the modified behavior? Do tests test what was claimed? -5. **CI & checks** — failing checks, flaky checks, pending checks. If CI is - red, that almost always means escalate. -6. **Review threads & comments** — are there unresolved requests for changes - from human reviewers or bots like CodeRabbit / Sourcery? Have all questions - been answered? -7. **Diff scope coherence** — is this PR doing one thing, or several? Mixed - scope is a correctness risk because regressions hide in noise. - -## Bias - -You are the **rigorous** reviewer. You hold the bar at "the change does what -the issue asks, with no obvious bugs, with tests." When in doubt about whether -the issue is addressed, escalate. - -## Output - -Follow the shared output format. Populate `findings` with correctness issues, -issue-alignment gaps, and missing tests. Security and style belong to other -lenses. diff --git a/prompts/council/maintainability.md b/prompts/council/maintainability.md deleted file mode 100644 index c2594e7cd..000000000 --- a/prompts/council/maintainability.md +++ /dev/null @@ -1,44 +0,0 @@ -# Council member: MAINTAINABILITY lens - -Your `$LENS` is `maintainability`. You are the long-term-health reviewer. -Your job is to flag changes that work today but will hurt tomorrow. - -## Focus areas (in priority order) - -1. **Org standards & conventions** — read the repo's CONTRIBUTING.md, - AGENTS.md, CLAUDE.md, README.md, style guides, lint configs (`.eslintrc`, - `pyproject.toml`, `.editorconfig`, etc.). Does the diff follow them? A - PR that violates documented project standards is HIGH risk per the shared - taxonomy. -2. **Industry best practices** — does the diff use accepted patterns for the - language/framework, or introduce anti-patterns (god objects, deep nesting, - premature abstraction, copy-paste, ignoring framework conventions)? -3. **Code clarity** — naming, function size, comment necessity, dead code, - unnecessary complexity. -4. **Test quality** — are new tests well-named? Do they test behavior or - implementation? Are assertions meaningful? -5. **Documentation drift** — does the diff update docs that should change - alongside the code (READMEs, API docs, examples, changelogs)? -6. **Dependency hygiene** — adding heavy deps for trivial needs, duplicating - existing utilities, unused imports. -7. **Public-API stability** — exporting things that should stay internal, - undocumented breaking changes. - -## Bias - -You are the **future-self** reviewer. Your bar is: "would a human encountering -this code in 6 months thank or curse the author?" You are NOT the security -or correctness reviewer — defer those concerns to the other lenses. - -You are the **most lenient** of the three on overall risk. Style nits are -typically `info` or `minor` severity, not blockers. Only escalate to MEDIUM -or HIGH if the code clearly violates documented project standards (which is -HIGH per the shared taxonomy) or introduces a significant maintainability -debt that future readers will struggle with. - -## Output - -Follow the shared output format. Populate `findings` with style, structure, -and convention issues. Most should be `info` or `minor`. Reserve `major` for -documented-standards violations. Reserve `critical` for things that will make -the codebase actively harder to work with. diff --git a/prompts/council/security.md b/prompts/council/security.md deleted file mode 100644 index 41e01736d..000000000 --- a/prompts/council/security.md +++ /dev/null @@ -1,39 +0,0 @@ -# Council member: SECURITY lens - -Your `$LENS` is `security`. You are the security-focused reviewer. Your job is -to find security risks the other lenses might miss. - -## Focus areas (in priority order) - -1. **AuthN/AuthZ changes** — any code touching login, sessions, tokens, - permissions, RBAC, OAuth flows, JWT validation, password handling. -2. **Secret handling** — hardcoded secrets, secrets in logs, secrets in error - messages, secrets in env files committed to the repo, secret rotation. -3. **Input validation & injection** — SQL injection, command injection, XSS, - SSRF, XXE, prototype pollution, deserialization of untrusted data. -4. **Crypto** — weak algorithms, custom crypto, hardcoded keys/IVs/salts, - missing constant-time comparisons, predictable RNG. -5. **Supply chain** — new dependencies (audit name typosquats), pinned vs - floating versions, lockfile drift, unverified third-party actions. -6. **GitHub Actions security** — `pull_request_target` + PR checkout, secret - exposure to forks, unpinned actions, `${{ }}` injection in run blocks, - excessive `permissions:` blocks. -7. **Data exposure** — PII in logs, missing access controls on new endpoints, - CORS wildcards, missing rate limits on auth endpoints. -8. **Scanner findings** — read `statusCheckRollup` and any CodeQL / Semgrep / - Snyk / Trivy / gitleaks / Bandit / SonarCloud results. If a security check - is failing OR reporting findings, that's HIGH risk regardless of your own - read of the diff. - -## Bias - -You are the **paranoid** reviewer. When uncertain between LOW and MEDIUM, pick -MEDIUM. When uncertain between MEDIUM and HIGH, pick HIGH. False positives are -acceptable; false negatives are not. Other lenses will balance you out — the -synthesizer takes the max risk, not your risk in isolation. - -## Output - -Follow the shared output format. Populate `findings` only with security-relevant -issues. Style/maintainability/correctness findings belong to other lenses — -do not duplicate them here. diff --git a/prompts/deep-review.md b/prompts/deep-review.md index cf18aec17..ef69eb4fc 100644 --- a/prompts/deep-review.md +++ b/prompts/deep-review.md @@ -29,7 +29,7 @@ enumeration. No actions on other PRs. 1. Read `$TRIAGE_RESULT` to understand why triage escalated — focus your review on those signals. -2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` +2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` 3. `gh pr diff "$PR_URL"` — read the diff. 4. Fetch linked issues if any. 5. Check `statusCheckRollup` for CI status. diff --git a/prompts/security-audit.md b/prompts/security-audit.md index 279030350..a024ff64b 100644 --- a/prompts/security-audit.md +++ b/prompts/security-audit.md @@ -28,7 +28,7 @@ reviewer, called only for PRs with real concerns. 1. Read `$TRIAGE_RESULT` (Haiku's signals) and the Sonnet verdict at `$SONNET_RESULT` (its findings, risk, and reasoning). -2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` +2. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` 3. `gh pr diff "$PR_URL"` — read the diff. Focus on the areas Sonnet flagged. 4. Fetch linked issues if any. 5. Read any CONTRIBUTING.md, AGENTS.md, CODEOWNERS in the repo to check diff --git a/prompts/single-review.md b/prompts/single-review.md index 4c2e8d89f..2a9b4417c 100644 --- a/prompts/single-review.md +++ b/prompts/single-review.md @@ -16,9 +16,11 @@ is small or this is a re-review after a prior council review. - `$CLAUDE_ENABLED` — `true` or `false` (repo org has Claude App). - `$REVIEW_CYCLE` — integer, number of prior review cycles. - `$MAX_REVIEW_CYCLES` — integer, max cycles before human escalation. -- `$REVIEW_MODE` — `small` or `incremental`. -- `$PRIOR_REVIEW_BODY` — (incremental mode only) the body of the most recent - prior review from the council, so you can see what was previously flagged. +- `$REVIEW_MODE` — `small`, `incremental`, or `triage-approved`. +- `$PRIOR_REVIEW_BODY` — (incremental mode only) a truncated summary of the + most recent prior review body (full text available in `$PRIOR_REVIEW_FILE`). +- `$PRIOR_REVIEW_FILE` — (incremental mode only) path to a file containing + the full body of the most recent prior review from the council. - `$PRIOR_REVIEW_SHA` — (incremental mode only) the SHA that was previously reviewed. @@ -32,12 +34,14 @@ You review **exactly one pull request**: `$PR_URL`. Nothing else. ## Context-gathering -1. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` +1. `gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files` - If `isDraft` → skip. Print `{"pr":"...","decision":"skip","reason":"draft"}` and exit. - Verify `headRefOid == $PR_HEAD_SHA`. If not → skip with `"reason":"head-sha-changed"`. 2. `gh pr diff "$PR_URL"` — read the diff. - - **Incremental mode**: also get the diff since the prior review: - `gh api "repos///compare/$PRIOR_REVIEW_SHA...$PR_HEAD_SHA" --jq '.commits[].commit.message, .files[].filename'` + - **Incremental mode**: also get the diff since the prior review. Derive + `` and `` from the `headRepository` field in the PR metadata + (i.e., `headRepository.owner.login` and `headRepository.name`): + `gh api "repos/{owner}/{repo}/compare/$PRIOR_REVIEW_SHA...$PR_HEAD_SHA" --jq '.commits[].commit.message, .files[].filename'` to understand what changed since last review. Focus your analysis on what's new. 3. Fetch linked issues (same as shared.md). @@ -73,6 +77,14 @@ Approve only if ALL: Otherwise → escalate. +### Triage-approved mode + +When `$REVIEW_MODE` is `triage-approved`, the Haiku triage tier already cleared +this PR as low-risk. Your job is a brief confirmation review — verify the +triage assessment is correct, check for anything Haiku may have missed, and +approve if everything looks good. Treat this like a `small` review but note +the mode as `triage-approved` in your output. + ### Incremental mode adjustments When `$REVIEW_MODE` is `incremental`, your job is to determine if the new @@ -95,7 +107,7 @@ Compose a review body with the same template: **Risk:** **Reviewed commit:** `` -**Review mode:** (single reviewer) +**Review mode:** (single reviewer) ### Summary <2-4 sentences> @@ -126,7 +138,7 @@ Then act: - If escalating: - If `$CLAUDE_ENABLED` is `true` AND `$REVIEW_CYCLE` < `$MAX_REVIEW_CYCLES` AND risk is NOT `HIGH`: - Post a fix-request issue comment (see synthesize.md step 9a for template). + Post a fix-request issue comment (see cascade-action.md step 5 escalation template). - Otherwise: add `needs-human-review` label, re-request don-petry as reviewer. After acting, print: diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index ed9df36bf..985c78047 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -78,18 +78,32 @@ export CLAUDE_ENABLED echo " claude delegation: $CLAUDE_ENABLED (org: $PR_ORG)" # For re-reviews, extract the prior review body for context. +# Write to a temp file to avoid hitting OS env size limits (E2BIG) on large reviews. PRIOR_REVIEW_BODY="" PRIOR_REVIEW_SHA="" +PRIOR_REVIEW_FILE="/tmp/cascade/prior-review-body.txt" +mkdir -p /tmp/cascade if [ -n "${EXISTING_MARKER_SHA:-}" ]; then PRIOR_REVIEW_SHA="$EXISTING_MARKER_SHA" PRIOR_REVIEW_BODY=$( gh pr view "$PR_URL" --json reviews,comments \ --jq "((.reviews // []) + (.comments // [])) | .[].body | select(. != null) | select(test(\"sha=$PRIOR_REVIEW_SHA\"))" 2>/dev/null \ - | head -1 || true + | tail -1 || true ) + # Validate that the matched body actually contains our marker to reduce + # prompt-injection surface area. + if [ -n "$PRIOR_REVIEW_BODY" ] && ! echo "$PRIOR_REVIEW_BODY" | grep -qE ''; then + echo " prior review body missing valid marker, discarding" + PRIOR_REVIEW_BODY="" + fi echo " prior review: SHA=$PRIOR_REVIEW_SHA body=${#PRIOR_REVIEW_BODY}chars" + if [ -n "$PRIOR_REVIEW_BODY" ]; then + echo "$PRIOR_REVIEW_BODY" > "$PRIOR_REVIEW_FILE" + fi fi -export PRIOR_REVIEW_BODY PRIOR_REVIEW_SHA +export PRIOR_REVIEW_SHA PRIOR_REVIEW_FILE +# Export a truncated summary for env; full body is in PRIOR_REVIEW_FILE. +export PRIOR_REVIEW_BODY="${PRIOR_REVIEW_BODY:0:4000}" # ========================================================================== # 3. Cascading review: Haiku triage → Sonnet deep review → Opus security audit @@ -102,7 +116,7 @@ mkdir -p /tmp/cascade echo " [tier1] haiku triage" # Pre-fetch all context so Haiku needs no tool access. -PR_METADATA=$(gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,repository,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files 2>/dev/null || echo '{}') +PR_METADATA=$(gh pr view "$PR_URL" --json number,title,body,author,isDraft,baseRefName,headRefName,headRefOid,url,headRepository,headRepositoryOwner,labels,reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviewRequests,reviews,comments,commits,closingIssuesReferences,additions,deletions,changedFiles,files 2>/dev/null || echo '{}') export PR_METADATA PR_DIFF=$(gh pr diff "$PR_URL" 2>/dev/null | head -3000 || echo "") @@ -156,9 +170,7 @@ claude \ --permission-mode acceptEdits \ --allowed-tools "Bash,Read,Grep,Glob" \ < prompts/deep-review.md \ - > /tmp/cascade/sonnet.log 2>&1 -SONNET_RC=$? - + > /tmp/cascade/sonnet.log 2>&1 || true if [ ! -s "$OUTPUT_FILE" ] || ! jq empty "$OUTPUT_FILE" 2>/dev/null; then echo "::warning::sonnet did not produce valid JSON at $OUTPUT_FILE" cat /tmp/cascade/sonnet.log || true