|
| 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 | +**How the fan-out runs — and why it keeps the caller's context fresh.** Stage 1 (scouting the |
| 16 | +work-list) runs inline in the main context; it is small and cheap. Stage 2 (the per-PR reviews) |
| 17 | +runs as a **`Workflow`** — a deterministic multi-agent orchestration — *not* as prose-directed |
| 18 | +subagent dispatch. Each PR's heavy work (a large diff, the three nested `review_pr` review agents, |
| 19 | +the full review body) is confined to a Workflow-spawned subagent's context; only a **small |
| 20 | +structured result per PR** (number, verdict, one-line rationale, review URL) returns to the caller. |
| 21 | +Making the fan-out a `Workflow` means that isolation is a property of the control flow, not |
| 22 | +something an executor must remember to do — so a large queue can't blow the main window on diff |
| 23 | +bodies and agent transcripts. (Caveat: per-PR *summaries* still accrete linearly in the caller, so |
| 24 | +"fresh" ≠ "constant" — a 200-PR run still returns 200 rows.) |
| 25 | + |
| 26 | +## Arguments |
| 27 | + |
| 28 | +Optional. |
| 29 | + |
| 30 | +- **No arguments** — review every PR that currently requests your review (the default query in |
| 31 | + Stage 1). |
| 32 | +- **Explicit list** — e.g. `#616 #612 590` — review exactly those PRs, skipping Stage-1 derivation. |
| 33 | +- **Natural-language filter** — e.g. "all open non-fork PRs with green CI", "everything P1 and |
| 34 | + above requesting my review" — translate to the appropriate `gh` search and gates in Stage 1. |
| 35 | + |
| 36 | +## Stage 1: Build the work-list |
| 37 | + |
| 38 | +Derive the candidate PRs. The default set is *ready-to-review, not mine, not yet approved by me, |
| 39 | +requesting my review, excluding forks*: |
| 40 | + |
| 41 | +```sh |
| 42 | +gh pr list --repo <owner>/<repo> \ |
| 43 | + --search "is:open is:pr draft:false -author:@me -review:approved review-requested:@me" \ |
| 44 | + --json number,isCrossRepository \ |
| 45 | + --jq '.[] | select(.isCrossRepository == false) | .number' |
| 46 | +``` |
| 47 | + |
| 48 | +- **Fork exclusion is client-side.** GitHub's issue/PR search has **no fork qualifier** — `is:fork` |
| 49 | + is a *repository*-search qualifier and is silently ignored in a PR search (it returns a superset, |
| 50 | + not an error). Filter on `isCrossRepository == false` (a PR whose head branch lives in a fork is |
| 51 | + cross-repository); `headRepositoryOwner.login` tells you *which* fork. |
| 52 | +- **Optional CI-green gate (only when the user asks for "green" / "passing" / "CI clean").** Keep a |
| 53 | + PR iff its checks are all terminal **and** all passed — i.e. `gh pr checks <n>` exits `0`. Do |
| 54 | + **not** pass `--json` to `gh pr checks` in the gate: with `--json` it always exits `0` and the |
| 55 | + exit code stops meaning anything (use `--json state,bucket` only for *reporting* the state). The |
| 56 | + gate is opt-in, not the default: a red or still-running PR is often exactly the one that needs a |
| 57 | + "request changes", so gating it out by default would hide the work. |
| 58 | +- **Announce what you drop.** For every PR removed by the CI gate (or any other filter), say so and |
| 59 | + why. Silent truncation reads as "reviewed everything" when it wasn't. |
| 60 | +- **Empty list** — if nothing matches, say so and stop. Never invent PR numbers. |
| 61 | +- **Transient by design** — submitting a review clears `review-requested:@me`, so re-running the |
| 62 | + default query later returns a *different* set. That is correct for a worklist; do not treat it as |
| 63 | + a bug or try to re-review already-reviewed PRs. |
| 64 | + |
| 65 | +Present the resolved list (with the reason any candidate was dropped) before fanning out. |
| 66 | + |
| 67 | +## Stage 2: Fan out via `Workflow` — one full `review_pr` per PR |
| 68 | + |
| 69 | +Run the per-PR reviews as a **`Workflow`**, not as inline subagent dispatch. The `Workflow` tool |
| 70 | +requires explicit opt-in — reaching for it here is warranted because the user invoked a batch |
| 71 | +review, and it is what keeps the diffs and nested-agent transcripts out of the caller's context |
| 72 | +(see the Persona note). For a **single** PR, skip the Workflow and just run `review_pr` directly — |
| 73 | +orchestration overhead isn't worth it for `N == 1`. |
| 74 | + |
| 75 | +**Pre-stage shared git state ONCE, inline, before invoking the Workflow** — worktree/branch |
| 76 | +creation mutates shared `.git` state and must not race across concurrent agents: |
| 77 | + |
| 78 | +1. `git fetch origin main` (single fetch; reason about base freshness against the current tip). |
| 79 | +2. Per PR: fetch its head (`git fetch origin pull/<n>/head:pr<n>head`), add a worktree on that |
| 80 | + ref, dump `gh pr diff <n>` to a scratch file, and capture the head SHA (`gh pr view <n> --json |
| 81 | + headRefOid`) and base branch (`--json baseRefName`). |
| 82 | + |
| 83 | +Assemble one descriptor per PR — `{ number, worktreePath, diffPath, headSha, baseRef }` — and pass |
| 84 | +the **array** as the Workflow's `args`. Then invoke `Workflow` with a self-contained script that: |
| 85 | + |
| 86 | +- reads the descriptors from `args` and fans out **one agent per PR concurrently** (`parallel()` |
| 87 | + over the descriptors, or `pipeline()` if you add a downstream stage); the tool caps real |
| 88 | + concurrency, so pass all PRs — they queue and drain automatically. |
| 89 | +- has each agent execute the full [`review_pr`](./review_pr.md) workflow for its PR and **submit** |
| 90 | + the review (inline suggestions + an Approve / Comment / Request-changes decision) via the Stage-3 |
| 91 | + mechanism. The agent prompt must include that PR's `worktreePath`, `diffPath`, `headSha` (the |
| 92 | + review `commit_id`), and `baseRef` — and **flag stacked PRs** whose `baseRef != main` (the diff |
| 93 | + is against the base branch, not `main`, and merge order matters). |
| 94 | +- carries the standing `review_pr` governance rule into each agent prompt: the gate is an |
| 95 | + **approved backing issue** (see |
| 96 | + [ADR-003](../../docs/decisions/ADR-003-contribution-governance.md)); a `pr/*` branch name is a |
| 97 | + de-facto-waived nit, **not** a blocker. Each agent must **read existing review threads first** |
| 98 | + and independently verify any prior blocking claim against current code before finalizing. |
| 99 | +- returns a **compact `schema`-validated result per PR** — `{ number, verdict, rationale, |
| 100 | + reviewUrl }` — and nothing larger. The `schema` is what guarantees only the summary crosses back |
| 101 | + to the caller; without it an agent's free-text return could drag a full review body into the main |
| 102 | + context, defeating the freshness goal. The Workflow's return value is the array of these results. |
| 103 | + |
| 104 | +Sketch (illustrative — adapt names/paths): |
| 105 | + |
| 106 | +```js |
| 107 | +export const meta = { |
| 108 | + name: 'review-prs', |
| 109 | + description: 'Review a set of PRs, one review_pr per PR, return compact verdicts', |
| 110 | + phases: [{ title: 'Review' }], |
| 111 | +} |
| 112 | +const RESULT = { type: 'object', required: ['number', 'verdict', 'reviewUrl'], properties: { |
| 113 | + number: { type: 'number' }, |
| 114 | + verdict: { enum: ['APPROVE', 'COMMENT', 'REQUEST_CHANGES'] }, |
| 115 | + rationale: { type: 'string' }, |
| 116 | + reviewUrl: { type: 'string' }, |
| 117 | +} } |
| 118 | +const results = await parallel(args.map((pr) => () => |
| 119 | + agent( |
| 120 | + `Run the /review_pr workflow for PR #${pr.number} and SUBMIT the review with inline ` + |
| 121 | + `suggestions. Worktree: ${pr.worktreePath}. Diff: ${pr.diffPath}. commit_id: ${pr.headSha}. ` + |
| 122 | + `Base: ${pr.baseRef}${pr.baseRef !== 'main' ? ' (STACKED — diff is vs the base branch)' : ''}. ` + |
| 123 | + `Governance: gate is an approved backing issue; a pr/* branch is a waived nit. Read existing ` + |
| 124 | + `review threads first and verify prior blocking claims. Submit via gh api (see review_pr).`, |
| 125 | + { label: `review:#${pr.number}`, phase: 'Review', schema: RESULT }, |
| 126 | + ))) |
| 127 | +return results.filter(Boolean) |
| 128 | +``` |
| 129 | + |
| 130 | +Scale the depth to the request inside each agent: a routine queue is one agent per PR running |
| 131 | +`review_pr` once; "audit these thoroughly" lets each agent invoke the deeper Stage-3 agent fan-out |
| 132 | +that `review_pr` itself prescribes. |
| 133 | + |
| 134 | +## Stage 3: Submit & report |
| 135 | + |
| 136 | +- **Submission mechanism** — the GitHub MCP `pull_request_review_write` tool may lack the required |
| 137 | + token scope (`Resource not accessible by personal access token`). When it does, submit via the |
| 138 | + `gh` CLI instead: |
| 139 | + |
| 140 | + ```sh |
| 141 | + gh api --method POST repos/<owner>/<repo>/pulls/<n>/reviews --input <review>.json |
| 142 | + ``` |
| 143 | + |
| 144 | + where `<review>.json` has `commit_id`, `event` (`APPROVE` | `COMMENT` | `REQUEST_CHANGES`), |
| 145 | + `body`, and `comments[]`. Inline comments must anchor to a line present on the **RIGHT** side of |
| 146 | + the diff, or the call returns `422` — move any such finding into the review body instead. |
| 147 | + Each Workflow agent submits its own review this way from inside its context — the review bodies |
| 148 | + never return to the caller (only the compact `schema` result does). |
| 149 | +- **Clean up** — after the Workflow returns, remove the per-PR worktrees and temporary branches |
| 150 | + created in Stage 2's pre-staging. |
| 151 | +- **Report** — assemble a summary table from the Workflow's returned results: |
| 152 | + **PR · verdict · one-line rationale · review URL**. Include the count reviewed and the count |
| 153 | + (and reasons) dropped in Stage 1. |
| 154 | + |
| 155 | +## Notes |
| 156 | + |
| 157 | +- This command is a thin orchestrator over `review_pr`. It intentionally does not duplicate the |
| 158 | + review stages, agent-invocation requirements, or output structure — refinements to the review bar |
| 159 | + land in [`review_pr`](./review_pr.md) and are inherited here automatically. |
| 160 | +- The Stage-2 fan-out uses the **`Workflow`** tool so context-freshness is enforced by the control |
| 161 | + flow rather than left to an executor following prose. This command *uses* `Workflow`; it does not |
| 162 | + modify orchestration infrastructure. Git pre-staging (worktrees/branches) stays inline before the |
| 163 | + Workflow because it mutates shared `.git` state and must not race across concurrent agents. |
0 commit comments