|
| 1 | +# Review Multiple Pull Requests in ABCA |
| 2 | + |
| 3 | +## Persona |
| 4 | + |
| 5 | +Hold the same bar as [`review_pr`](./review_pr.md): a **Principal AWS Solutions Architect** who |
| 6 | +is direct, specific, justifies every concern with a concrete risk and a `file:line`, distinguishes |
| 7 | +blocking issues from nits, and never rubber-stamps. This command orchestrates that single-PR |
| 8 | +review across a *set* of PRs — it does **not** restate the review process. Each PR is reviewed by |
| 9 | +the full `review_pr` workflow, so the review bar stays single-sourced. |
| 10 | + |
| 11 | +This is **ABCA (Autonomous Background Coding Agents on AWS)** — a self-hosted platform for |
| 12 | +background coding agents. Bounded blast radius applies to the *reviewer* too: this command can |
| 13 | +fan out many concurrent sub-reviews, so it is explicit about scope, cost, and what it skips. |
| 14 | + |
| 15 | +## Arguments |
| 16 | + |
| 17 | +Optional. |
| 18 | + |
| 19 | +- **No arguments** — review every PR that currently requests your review (the default query in |
| 20 | + Stage 1). |
| 21 | +- **Explicit list** — e.g. `#616 #612 590` — review exactly those PRs, skipping Stage-1 derivation. |
| 22 | +- **Natural-language filter** — e.g. "all open non-fork PRs with green CI", "everything P1 and |
| 23 | + above requesting my review" — translate to the appropriate `gh` search and gates in Stage 1. |
| 24 | + |
| 25 | +## Stage 1: Build the work-list |
| 26 | + |
| 27 | +Derive the candidate PRs. The default set is *ready-to-review, not mine, not yet approved by me, |
| 28 | +requesting my review, excluding forks*: |
| 29 | + |
| 30 | +```sh |
| 31 | +gh pr list --repo <owner>/<repo> \ |
| 32 | + --search "is:open is:pr draft:false -author:@me -review:approved review-requested:@me" \ |
| 33 | + --json number,isCrossRepository \ |
| 34 | + --jq '.[] | select(.isCrossRepository == false) | .number' |
| 35 | +``` |
| 36 | + |
| 37 | +- **Fork exclusion is client-side.** GitHub's issue/PR search has **no fork qualifier** — `is:fork` |
| 38 | + is a *repository*-search qualifier and is silently ignored in a PR search (it returns a superset, |
| 39 | + not an error). Filter on `isCrossRepository == false` (a PR whose head branch lives in a fork is |
| 40 | + cross-repository); `headRepositoryOwner.login` tells you *which* fork. |
| 41 | +- **Optional CI-green gate (only when the user asks for "green" / "passing" / "CI clean").** Keep a |
| 42 | + PR iff its checks are all terminal **and** all passed — i.e. `gh pr checks <n>` exits `0`. Do |
| 43 | + **not** pass `--json` to `gh pr checks` in the gate: with `--json` it always exits `0` and the |
| 44 | + exit code stops meaning anything (use `--json state,bucket` only for *reporting* the state). The |
| 45 | + gate is opt-in, not the default: a red or still-running PR is often exactly the one that needs a |
| 46 | + "request changes", so gating it out by default would hide the work. |
| 47 | +- **Announce what you drop.** For every PR removed by the CI gate (or any other filter), say so and |
| 48 | + why. Silent truncation reads as "reviewed everything" when it wasn't. |
| 49 | +- **Empty list** — if nothing matches, say so and stop. Never invent PR numbers. |
| 50 | +- **Transient by design** — submitting a review clears `review-requested:@me`, so re-running the |
| 51 | + default query later returns a *different* set. That is correct for a worklist; do not treat it as |
| 52 | + a bug or try to re-review already-reviewed PRs. |
| 53 | + |
| 54 | +Present the resolved list (with the reason any candidate was dropped) before fanning out. |
| 55 | + |
| 56 | +## Stage 2: Fan out — one full `review_pr` per PR |
| 57 | + |
| 58 | +**Pre-stage shared git state ONCE** to avoid concurrent-git lock contention across sub-reviews: |
| 59 | + |
| 60 | +1. `git fetch origin main` (single fetch; reason about base freshness against the current tip). |
| 61 | +2. Per PR: fetch its head (`git fetch origin pull/<n>/head:pr<n>head`), add a worktree on that |
| 62 | + ref, and dump `gh pr diff <n>` to a scratch file. |
| 63 | + |
| 64 | +Then dispatch **one sub-reviewer per PR in a single concurrent batch**, each instructed to execute |
| 65 | +the full [`review_pr`](./review_pr.md) workflow for its PR and **submit** the review with inline |
| 66 | +suggestions and an Approve / Comment / Request-changes decision. Give each sub-reviewer: |
| 67 | + |
| 68 | +- the **PR number**, its **worktree path**, its **diff path**, the **head SHA** (the review |
| 69 | + `commit_id`), and its **base branch** — explicitly flag **stacked PRs** whose base is not `main` |
| 70 | + (the diff is then against the base branch, not `main`, and merge order matters). |
| 71 | +- the standing governance rule carried from `review_pr`: the gate is an **approved backing issue** |
| 72 | + (see [ADR-003](../../docs/decisions/ADR-003-contribution-governance.md)); a `pr/*` branch name is |
| 73 | + a de-facto-waived nit in this repo, **not** a blocker. |
| 74 | +- the instruction to **read existing review threads first** and independently verify any prior |
| 75 | + blocking claim against the current code before finalizing. |
| 76 | + |
| 77 | +Scale the depth to the request: a routine queue gets one sub-reviewer per PR; "audit these |
| 78 | +thoroughly" warrants the deeper Stage-3 agent fan-out that `review_pr` itself prescribes. |
| 79 | + |
| 80 | +## Stage 3: Submit & report |
| 81 | + |
| 82 | +- **Submission mechanism** — the GitHub MCP `pull_request_review_write` tool may lack the required |
| 83 | + token scope (`Resource not accessible by personal access token`). When it does, submit via the |
| 84 | + `gh` CLI instead: |
| 85 | + |
| 86 | + ```sh |
| 87 | + gh api --method POST repos/<owner>/<repo>/pulls/<n>/reviews --input <review>.json |
| 88 | + ``` |
| 89 | + |
| 90 | + where `<review>.json` has `commit_id`, `event` (`APPROVE` | `COMMENT` | `REQUEST_CHANGES`), |
| 91 | + `body`, and `comments[]`. Inline comments must anchor to a line present on the **RIGHT** side of |
| 92 | + the diff, or the call returns `422` — move any such finding into the review body instead. |
| 93 | +- **Clean up** — remove the per-PR worktrees and temporary branches created in Stage 2. |
| 94 | +- **Report** — return a summary table: **PR · verdict · one-line rationale · review URL**. Include |
| 95 | + the count reviewed and the count (and reasons) dropped in Stage 1. |
| 96 | + |
| 97 | +## Notes |
| 98 | + |
| 99 | +- This command is a thin orchestrator over `review_pr`. It intentionally does not duplicate the |
| 100 | + review stages, agent-invocation requirements, or output structure — refinements to the review bar |
| 101 | + land in [`review_pr`](./review_pr.md) and are inherited here automatically. |
0 commit comments