Skip to content

Commit 9853da8

Browse files
committed
Added PR reviewing to skill
1 parent 6db8e7d commit 9853da8

4 files changed

Lines changed: 138 additions & 9 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
name: pr-summary-reviewer
3+
description: Produces an overall description of what a pull request does and an opinion on whether it is a good change. External-PR-only — runs when `/review-areas` is invoked with a PR number. Distinct lens from the area and cross-cutting reviewers: those answer "is each piece correct?"; this one answers "what is the PR, and on balance is it the right change?"
4+
tools: Read, Grep, Glob, Bash
5+
model: inherit
6+
---
7+
8+
You are the PR summary reviewer for the MongoDB C# driver. You run only in external PR mode of `/review-areas` and produce a holistic description of the pull request plus an opinion on its merit.
9+
10+
## Authoritative context
11+
12+
Read the root `AGENTS.md`. Skim per-area `AGENTS.md` files only if a touched directory has one and the PR's stated rationale needs cross-checking against it.
13+
14+
The parent agent will pass you:
15+
- The PR number, title, URL, base ref, and head ref.
16+
- The full list of changed files in the diff range.
17+
18+
Pull the PR body and linked-issue context yourself:
19+
- `gh pr view <PR#> --json body,labels,additions,deletions,changedFiles,author` — this gives you the author's stated rationale.
20+
- If the title or body references a JIRA ticket (`CSHARP-NNNN`) or GitHub issue, that's useful context for *why* the PR exists; you don't need to fetch JIRA, but note the reference.
21+
22+
## What this reviewer does
23+
24+
Two things, in order:
25+
26+
1. **Describe the PR.** A short paragraph (3–6 sentences) covering:
27+
- What functional area(s) the PR touches.
28+
- What the change actually does at a behavioral level (not file-by-file — synthesize across the diff).
29+
- The stated motivation from the PR body / linked ticket, if any.
30+
- The scope and shape: bug fix vs feature vs refactor vs docs; size in rough terms (one-line fix, focused change, sprawling); whether tests accompany the code change.
31+
32+
2. **Judge the change.** A short paragraph (3–6 sentences) covering:
33+
- Is the problem real and worth fixing? (If the PR body claims a bug, does the diff actually demonstrate the bug existed?)
34+
- Is the chosen approach reasonable, or is there an obviously better one?
35+
- Is the scope right — too narrow (misses adjacent broken cases) or too wide (drags in unrelated changes)?
36+
- Are tests adequate for the kind of change? (Bug fix → regression test; feature → coverage of golden path + at least one edge case; refactor → existing tests still meaningful.)
37+
- Any red flags about timing, dependencies, or surface that the per-area reviewers wouldn't catch because they look only at their slice.
38+
39+
## What this reviewer does NOT do
40+
41+
- Do not duplicate the per-area or cross-cutting reviewers. You are not auditing line-by-line for bugs, API breaks, async hygiene, or security. If you spot one of those, mention it briefly and trust that the relevant reviewer will catch it in detail.
42+
- Do not paste the diff back.
43+
- Do not run tests — you are giving an opinion, not validating behavior.
44+
45+
## Output shape
46+
47+
Produce exactly this, no preamble:
48+
49+
**Description**: one paragraph as specified above.
50+
51+
**Assessment**: one paragraph as specified above.
52+
53+
**Verdict**: one of `looks good`, `mixed`, `concerns`.
54+
- `looks good` = problem is real, approach is sound, scope and tests are appropriate.
55+
- `mixed` = the change is defensible but has notable weaknesses (e.g. missing test, debatable approach, scope creep) that a reviewer should call out.
56+
- `concerns` = the premise, approach, or scope looks wrong in a way the user should weigh before merging. This is your equivalent of the area reviewers' `escalate`.
57+
58+
Hard limit: 500 words total.

.claude/commands/review-areas.md

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
---
2-
description: Fan-out review of the current branch — runs each per-area reviewer over the files it owns and aggregates findings.
3-
argument-hint: [--all] [--model opus|sonnet|haiku] [<base-ref> [<head-ref>]]
2+
description: Fan-out review of the current branch or an external PR — runs each per-area reviewer over the files it owns and aggregates findings.
3+
argument-hint: [--all] [--model opus|sonnet|haiku] [<PR#> | <base-ref> [<head-ref>]]
44
---
55

66
# /review-areas
77

8-
Run the per-area reviewer sub-agents (`.claude/agents/*-reviewer.md`) over the current branch's changes in parallel and produce one consolidated report.
8+
Run the per-area reviewer sub-agents (`.claude/agents/*-reviewer.md`) over a diff range — either the current branch's changes (local range mode) or an external PR's changes (external PR mode) — in parallel, and produce one consolidated report.
99

1010
User args: `$ARGUMENTS`
1111

@@ -14,12 +14,26 @@ User args: `$ARGUMENTS`
1414
Parse `$ARGUMENTS`:
1515
- If `--all` is present, queue every reviewer in the table below regardless of diff. Skip step 2's filtering.
1616
- If `--model <name>` is present, capture `<name>` (must be one of `opus`, `sonnet`, `haiku`) and pass it on every `Agent` dispatch in step 3. If absent, omit the `model` parameter so each reviewer falls back to its frontmatter setting (currently `inherit` for all of them, which means the parent session's model).
17-
- Collect the remaining non-flag tokens in order:
17+
- Examine the remaining non-flag tokens:
18+
19+
**External PR mode** — if a token looks like a PR number (a bare integer such as `1234`, or a `#`-prefixed integer such as `#1234`), treat this as an external PR review:
20+
1. Run `gh pr view <PR#> --json number,title,url,baseRefName,headRefOid` and capture the JSON.
21+
2. Parse `<owner>/<repo>` from the PR URL (e.g. `https://github.com/mongodb/mongo-csharp-driver/pull/1991``mongodb/mongo-csharp-driver`).
22+
3. Run `git remote -v` and find the remote whose fetch URL contains `<owner>/<repo>` (match is case-insensitive and works for both HTTPS and SSH URLs). If multiple remotes match, pick the first. If none match, fall back to trying `upstream` then `origin`.
23+
4. Run `git fetch <remote> refs/pull/<PR#>/head` to bring the PR's head commit into `FETCH_HEAD`.
24+
5. Capture the head SHA: `git rev-parse FETCH_HEAD`.
25+
6. Ensure the base branch is locally available: `git fetch <remote> <baseRefName>` (safe to run even if already present). The tracking ref will be `<remote>/<baseRefName>`.
26+
7. Set **base ref** = `<remote>/<baseRefName>` and **head ref** = the SHA from step 5.
27+
8. Record the PR metadata (number, title, URL) and the remote name for display in the step 4 report header.
28+
29+
**Local range mode** — otherwise, collect the remaining non-flag tokens in order:
1830
- First token → **base ref** (default: `main`)
1931
- Second token → **head ref** (default: `HEAD`)
2032

2133
Use `<base>...<head>` as the diff range throughout (three-dot syntax finds the merge base). Examples:
2234
- `/review-areas``main...HEAD`
35+
- `/review-areas 1234` → external PR #1234 (fetches and diffs against its base branch)
36+
- `/review-areas #1234` → same as above
2337
- `/review-areas HEAD~3``HEAD~3...HEAD` (last 3 commits only)
2438
- `/review-areas abc123 def456``abc123...def456` (arbitrary commit range)
2539
- `/review-areas --all HEAD~5 HEAD~2` → all reviewers, commits HEAD~5 through HEAD~2
@@ -47,7 +61,7 @@ Match each changed file against the table. A file may match more than one review
4761
| `geojson-reviewer` | `src/MongoDB.Driver/GeoJsonObjectModel/**` |
4862
| `spec-conformance-reviewer` | `tests/MongoDB.Driver.Tests/Specifications/**`; `tests/MongoDB.Driver.TestHelpers/**`; `tests/MongoDB.Bson.TestHelpers/**`; `tests/MongoDB.TestHelpers/**` |
4963

50-
**Meta-mapping for reviewer definitions:** a change to `.claude/agents/<name>-reviewer.md` is reviewed by that same reviewer (e.g. `.claude/agents/bson-reviewer.md``bson-reviewer`, `.claude/agents/security-reviewer.md``security-reviewer`). The reviewer is best placed to judge whether its own brief still accurately characterizes the area. Cross-cutting reviewer definitions map to themselves the same way.
64+
**Meta-mapping for reviewer definitions:** a change to `.claude/agents/<name>-reviewer.md` is reviewed by that same reviewer (e.g. `.claude/agents/bson-reviewer.md``bson-reviewer`, `.claude/agents/security-reviewer.md``security-reviewer`). The reviewer is best placed to judge whether its own brief still accurately characterizes the area. Cross-cutting reviewer definitions map to themselves the same way. Exception: `pr-summary-reviewer` only runs in external PR mode, so a local-range change to `.claude/agents/pr-summary-reviewer.md` won't be self-reviewed — that file's review happens when the PR is run through `/review-areas <PR#>`.
5165

5266
If new area reviewers are added under `.claude/agents/`, update this table.
5367

@@ -61,9 +75,13 @@ These three reviewers run on every invocation of `/review-areas`, regardless of
6175
| `api-stability-reviewer` | public surface / SemVer breaks |
6276
| `async-reviewer` | async/threading hygiene |
6377

78+
## PR-summary reviewer (external PR mode only)
79+
80+
In external PR mode, also dispatch the `pr-summary-reviewer` agent. It produces a holistic description of the PR (what it does, why) plus an opinion on whether it's a good change. It runs in parallel with everything else, and its output goes at the top of the consolidated report (before `## Summary`). Skip it in local range mode — there is no PR body to read.
81+
6482
## Step 3 — Dispatch reviewers in parallel
6583

66-
**Critical**: emit a single assistant message containing one `Agent` tool-use block per dispatched reviewer — both the matched area reviewers from step 2 *and* all three cross-cutting reviewers. Multiple `Agent` calls in the same message run concurrently; sequential calls do not. Use `subagent_type: <reviewer-name>` for each. If `--model <name>` was parsed in step 1, set `model: <name>` on every block; otherwise omit the field.
84+
**Critical**: emit a single assistant message containing one `Agent` tool-use block per dispatched reviewer — the matched area reviewers from step 2, all three cross-cutting reviewers, *and* (in external PR mode) `pr-summary-reviewer`. Multiple `Agent` calls in the same message run concurrently; sequential calls do not. Use `subagent_type: <reviewer-name>` for each. If `--model <name>` was parsed in step 1, set `model: <name>` on every block; otherwise omit the field.
6785

6886
### Area-reviewer prompt template
6987

@@ -110,15 +128,43 @@ Use `git diff <base>...<head>` to see the full picture and `git diff <base>...<h
110128
Produce a report in exactly the same shape as the area reviewers (Verdict / Findings / Tests run; same format and 400-word cap; same verdict semantics). Findings must be specific to your concern — do not duplicate what an area reviewer would catch.
111129
```
112130

131+
### PR-summary prompt template (external PR mode only)
132+
133+
For `pr-summary-reviewer`, use this template (substitute the PR fields, `<base>`, `<head>`, and the *full* changed-file list):
134+
135+
```
136+
You are running as the PR summary reviewer in a multi-area branch review of an external pull request.
137+
138+
PR: #<number> — <title>
139+
URL: <url>
140+
Diff range: <base>...<head>
141+
142+
All files changed in this range:
143+
- <file1>
144+
- <file2>
145+
146+
147+
Pull the PR body yourself with `gh pr view <number> --json body,labels,additions,deletions,changedFiles,author` to get the author's stated rationale. Use `git diff <base>...<head>` for the diff and read files at their current state where context matters.
148+
149+
Produce a report in exactly the shape specified in your agent definition (Description / Assessment / Verdict; 500-word cap). Do not duplicate the per-area or cross-cutting reviewers' work.
150+
```
151+
113152
## Step 4 — Aggregate
114153

115-
After all reviewers return, produce one consolidated report in this shape and show only this to the user (do not paste raw sub-agent transcripts):
154+
After all reviewers return, produce one consolidated report in this shape and show only this to the user (do not paste raw sub-agent transcripts).
155+
156+
For the report heading:
157+
- **External PR mode**: `# PR review: #<number> — <title>` followed by the PR URL and `diff range: <base>...<head>` on separate lines.
158+
- **Local range mode**: `# Branch review: <base>...<head>`
116159

117160
```
118-
# Branch review: <base>...<head>
161+
# <heading from above>
162+
163+
## PR summary
164+
(External PR mode only — paste the `pr-summary-reviewer`'s Description, Assessment, and Verdict verbatim. Omit this section in local range mode.)
119165
120166
## Summary
121-
N reviewers ran (M area + 3 cross-cutting). X approved, Y flagged, Z escalated.
167+
N reviewers ran (M area + 3 cross-cutting [+ 1 PR summary in external PR mode]). X approved, Y flagged, Z escalated. (PR-summary verdict is not counted in those totals — it's a separate lens.)
122168
123169
## Escalations
124170
For each `escalate` verdict from any reviewer — reviewer name, then its findings verbatim. Omit section if none.
@@ -133,8 +179,15 @@ For each area reviewer that ran: verdict + findings as bullets if `flag`, or `<r
133179
Files from the diff that didn't match any area reviewer. (Cross-cutters cover the diff regardless, so this is purely a coverage signal for the area mapping.) Omit section if none.
134180
```
135181

182+
**External PR mode only — save to file**: after composing the report, determine the output filename:
183+
1. List files in the current directory matching `review<number>[a-z].md` (e.g. `review1991a.md`, `review1991b.md`).
184+
2. Find the lowest letter (`a``z`) not already taken; use `a` if none exist yet.
185+
3. Write the full report to `review<number><letter>.md` in the current directory using the Write tool.
186+
4. Tell the user the filename at the end of your response (one line, e.g. `Saved to review1991a.md`).
187+
136188
## Notes
137189

138190
- Reviewers are non-mutating with respect to the source tree: each one is configured with `tools: Read, Grep, Glob, Bash` and has no `Edit` / `Write` / `Patch` tool, so they cannot modify files. They *can* still execute shell commands via `Bash` (e.g. `dotnet test`, `git diff`), which is not strictly "read-only"; scope reviewer Bash usage to inspection commands and treat any suggested fix as something the parent applies, not the reviewer.
139191
- A diff against a feature branch's own base is the typical use; pass an explicit `<base-ref>` for non-`main` cases (e.g. `/review-areas release/3.0`). Pass a second positional arg to cap the end of the range (e.g. `/review-areas HEAD~5 HEAD~1` to review only the last 5 commits excluding the most recent one).
192+
- In external PR mode, the remote is inferred from the PR's repo URL, so PRs on forks (`upstream`, `origin`, or any named remote) are handled automatically without any extra flags.
140193
- Test-running by reviewers is opportunistic — most won't, given how slow `dotnet test` is. Don't insist.

AGENTS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,11 @@ These reviewers have no per-area `AGENTS.md`; they apply a single hygiene lens a
8989
| Security: secrets, TLS/crypto, redaction, deserialization safety | `security-reviewer` |
9090
| Public API / SemVer | `api-stability-reviewer` |
9191
| Async/threading hygiene | `async-reviewer` |
92+
93+
## PR-summary reviewer (external PR mode only)
94+
95+
When `/review-areas` is invoked with a PR number, an additional reviewer runs alongside the others:
96+
97+
| Concern | Reviewer |
98+
|---|---|
99+
| Holistic "what does this PR do, and is it a good change?" — reads the PR body and the full diff | `pr-summary-reviewer` |

docs/agents-architecture.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ These reviewers have no per-area `AGENTS.md` and no path-scoping. They apply a s
4848

4949
Cross-cutting reviewers always run alongside whatever area reviewers are dispatched. They complement the area reviewers rather than replacing them: area reviewers own correctness of the logic; cross-cutting reviewers own their specific hygiene lens across the whole diff.
5050

51+
## PR-summary reviewer (external PR mode only)
52+
53+
One additional reviewer runs only when `/review-areas` is invoked with a PR number (external PR mode). It does not run for local branch reviews because there is no PR body to read.
54+
55+
| Sub-agent | Concern |
56+
|---|---|
57+
| `pr-summary-reviewer` | Holistic "what does this PR do, and is it a good change?" — pulls the PR body via `gh pr view`, synthesizes across the whole diff, and gives an opinion (`looks good` / `mixed` / `concerns`). Distinct from the per-area and cross-cutting reviewers, which look at correctness of individual pieces; this one judges the PR as a whole. |
58+
59+
It runs in parallel with the area and cross-cutting reviewers, and its output is rendered at the top of the consolidated report so the reader sees the high-level take before the line-level findings.
60+
5161
## Boundary decisions worth knowing
5262

5363
- **`src/MongoDB.Driver/AGENTS.md` is a router**, not a deep doc. Keep it short and section-organized — substantive content there only for code that actually lives at the directory root (facades, builders, aggregation/change-stream fluent API). Deeper subdirectory `AGENTS.md` files always *layer* additional context on top; nothing is lost.

0 commit comments

Comments
 (0)