agentleFS
jamestexasjamestexas/agents/skills/pr-review/SKILL.md

pr-review

Skill2 starsChanged 6 months agoApache-2.0View on GitHub

Give or respond to PR reviews with structural awareness via mache. Review mode: review-prep diagrams + impact analysis, parallel analysis agents, one consolidated GitHub review. Respond mode: address reviewer comments.

  • Commits and pushes

195 other projects use a version of this.

Adopted by

Most recently taken up by

Used with

Projects with it also use oss-pr, pr-automation and 10 more.

Other files people use alongside this one.

Files found in the same public projects as a version of this one, more often than they turn up anywhere else. Each project counts once, large collections count for less, and this file's own repository is left out.
  • CES-LtdSkill

    oss-pr

    Use when creating a pull request, after committing changes, or when user invokes /oss-pr. Covers branch management, quality checks, commit, push, and PR creation.

    Also in 3 projects that use this

  • CES-LtdSkill

    pr-automation

    PR Automation Orchestrator: poll open PRs, check CI, run review, fix, and merge eligible PRs. Use when: (1) Invoked by daemon via scripts/pr-automation.sh, (2) User says "/pr-automation".

    Also in 3 projects that use this

  • jtenniswoodSkill

    expert-reset

    Reimagine this repository from first principles using the current product and user experience as the target. Use when the user says "/expert-reset", asks how the project should be rebuilt if starting again, wants bold architecture or component-structure recommendations, or wants a research-only reset plan unconstrained by current implementation details.

    Also in 3 projects that use this

  • jtenniswoodSkill

    updates

    Summarize user-facing project updates from the last 7 days, ordered by importance and written as one-sentence "Feature: description" entries. Use when the user says "/updates", asks what changed recently, or wants a concise list of recent features, device support, fixes that users will notice, or other product value.

    Also in 3 projects that use this

  • openchamberSkill

    clack-cli-patterns

    Use when creating or modifying OpenChamber CLI commands, prompts, terminal output, non-TTY behavior, `--quiet`, or `--json` behavior.

    Also in 3 projects that use this

  • openchamberSkill

    drag-to-reorder

    Use when implementing or modifying OpenChamber sortable or drag-to-reorder behavior, especially `@dnd-kit`, touch/mobile interactions, variable-width items, or wrapping layouts.

    Also in 3 projects that use this

  • +6 more from openchamber

The file

Show its rules and text

We split this skill into the rules it gives an agent

35 rules. 0 are common, 25 are distinctive, and 10 are very specific to this project.

  • 0

    Common

    Most of the 178 edited versions we read keep these.

  • 25

    Distinctive

    This author's own choices. Worth a look.

  • 10

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  • could be automatic: a setting could enforce it
  1. Distinctive:

    Ask: "Structural pre-flight complete.

    line 45
  2. Distinctive:

    If the user says no or wants to inspect further, stop here.

    line 46
  3. Very specific:

    If review-prep returns status: unavailable:

    line 50
  4. Distinctive:

    Do NOT block the review.

    line 53
  5. Distinctive:

    If PR number provided:

    line 61
  6. Distinctive:

    If Linear ticket ID provided (format: ABC-123):

    line 66
  7. Distinctive:

    If nothing provided, detect from current branch:

    line 71
  8. Distinctive:

    Ask user to confirm if ambiguous.

    line 77
  9. Very specific:

    If user specified --review or --respond, use that mode.

    line 99
  10. Distinctive:

    Respond mode: if: the current git user is the PR author AND there are unaddressed review comments from others

    line 102
  11. Distinctive:

    Review mode: if: the current git user is NOT the PR author, OR there are no unaddressed comments, OR the user's prompt indicates they want …

    line 103
  12. Distinctive:

    Read the full diff from 1.2

    line 119
  13. Distinctive:

    Read the PR description to understand scope and intent

    line 121
  14. Distinctive:

    G1 — Gather the raw material: Run the context script rather than re-deriving by hand what it already collects:

    line 134
  15. Very specific:

    If a section comes back unavailable, say so in the review.

    line 158
  16. Very specific:

    If a language-specific standards skill exists (e.g., /go-standards), invoke it on all changed files.

    line 308
  17. Very specific:

    If STRUCTURAL_CONTEXT includes a non-trivial drift observation OR any high_blast_radius symbol:

    line 351
  18. Very specific:

    Only confirmed and revised findings continue to R.7.

    line 390
  19. Very specific:

    If STRUCTURAL_CONTEXT is non-empty, display the emergent diagram one final time alongside the compiled review for reference.

    line 446
  20. Very specific:

    If there are disagreements between agents, include a Disagreements section:

    line 450
  21. Distinctive:

    If user approves, post via:

    line 471
  22. Distinctive:

    If the referenced lines were already changed in a subsequent commit, mark the comment as "likely addressed" and verify by reading the …

    line 504
  23. Distinctive:

    Before implementing fixes, run a quick verification pass on non-trivial comments:

    line 508
  24. Distinctive:

    Create a task list grouped by file/component:

    line 524
  25. Distinctive:

    Start with already-addressed items (just confirm, no code change needed)

    line 557
  26. Distinctive:

    Read files before editing

    line 561
  27. Distinctive:

    Only fix what was requested — no scope creep

    line 562
  28. Very specific:

    Read the @@ hunks to get the line range.

    line 625
  29. Distinctive:

    If the fix spans multiple hunks in the same file, link the most relevant one.

    line 630
  30. Distinctive:

    Refusal condition — a template is a gate, not a mould: If you cannot build the permalink for shapes 1 or 2, the reply is not posted.

    line 678
  31. Very specific:

    HARD GATE: Before presenting responses to the user, scan every response body for github.com/.../blob/.

    line 693
  32. Distinctive:

    Ask: "Ready to post these responses?

    line 695
  33. Distinctive:

    If a Linear ticket is linked:

    line 715
  34. Distinctive:

    Git push fails: Check if branch is behind remote.

    line 735
  35. Distinctive:

    Always confirm with user before posting reviews or pushing code

    line 815

“Common” is measured: kept by at least 6 in 10 of the 178 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

PR Review

Give or respond to pull request reviews. Review mode runs parallel analysis agents through three lenses — convention compliance, behavioral correctness, and infrastructure — then synthesizes findings with explicit disagreement handling.

Arguments

$ARGUMENTS

Arguments can be:

  • PR number: 123 or owner/repo#123
  • Linear ticket: ENG-123 (finds associated PR)
  • Nothing (auto-detect from current branch)
  • Explicit mode: --review 123 (give review) or --respond 123 (address comments)

Phase 0: Structural Pre-flight

Runs before analysis agents. Output is local — not posted to GitHub.

0.1 Run review-prep

Invoke /review-prep $PR_NUM (or owner/repo#$PR_NUM if cross-repo).

Store the full ## Structural Pre-flight output as STRUCTURAL_CONTEXT.

0.2 Show reviewer

Display to the user now (before agents run):

  • Both diagrams (spec-driven + emergent, or whichever are available)
  • Drift analysis
  • Insights block
  • High blast-radius symbols list

Ask: "Structural pre-flight complete. Continue with full review?" If the user says no or wants to inspect further, stop here.

0.3 Handle unavailable

If review-prep returns status: unavailable:

  • Note: "Structural analysis unavailable — mache could not index this repo. Proceeding with standard review."
  • Set STRUCTURAL_CONTEXT="" and continue to Phase 1.
  • Do NOT block the review.

Phase 1: Discovery

1.1 Determine the PR

If PR number provided:

PR_NUM="$1"

If Linear ticket ID provided (format: ABC-123):

# Use Linear MCP to get issue and extract PR link

If nothing provided, detect from current branch:

BRANCH=$(git branch --show-current)
gh pr list --head "$BRANCH" --json number,title,url

Ask user to confirm if ambiguous.

1.2 Fetch PR metadata

# PR details
gh pr view $PR_NUM --json title,body,author,files,baseRefName,headRefName

# Full diff
gh pr diff $PR_NUM

# Existing review comments
gh api repos/{owner}/{repo}/pulls/$PR_NUM/comments \
  --jq '.[] | {path: .path, line: .line, body: .body, id: .id, user: .user.login, in_reply_to_id: .in_reply_to_id}'

# Review submissions (approvals, change requests)
gh api repos/{owner}/{repo}/pulls/$PR_NUM/reviews \
  --jq '.[] | {id: .id, user: .user.login, state: .state, body: .body}'

1.3 Detect mode

If user specified --review or --respond, use that mode.

Otherwise auto-detect:

  • Respond mode if: the current git user is the PR author AND there are unaddressed review comments from others
  • Review mode if: the current git user is NOT the PR author, OR there are no unaddressed comments, OR the user's prompt indicates they want to review

Present the detected mode and ask user to confirm before proceeding.

1.4 Check for related Linear ticket (optional)

Extract ticket ID from PR description or branch name. If found, fetch context via Linear MCP.


Review Mode

You are giving a review of someone else's (or your own) PR.

R.1 Read the diff and existing comments

  • Read the full diff from 1.2
  • Note any existing review comments (resolved, in-progress, or unaddressed) to avoid duplicating feedback
  • Read the PR description to understand scope and intent
  • Note the stated scope — you'll check for both scope creep AND understated scope later

R.2 Generation — produce the candidate set

Everything that produces candidates lives in this phase. It is not finished when the agents come back; it is finished when every review dimension has a named result.

Ordering: G1 and G2 run now, because what they return is input to the lenses. G3 is the lenses themselves (R.3 and R.4). G4 is the exit gate — fill it once the lenses have returned, and before synthesis in R.5.

G1 — Gather the raw material. Run the context script rather than re-deriving by hand what it already collects:

pr-context.sh --pr owner/repo#$PR_NUM > /tmp/pr-ctx-$PR_NUM.json

It emits one JSON document (schema: pr-context/v1) in which every section carries a status — ok / empty / unavailable / skipped — so "we looked and there is nothing here" is never confusable with "we did not look." It is read-only and forms no findings; the judgment stays here. Add --similar 0 to skip the slowest section when you need it fast.

G2 — Mine what came back. Each section is a different kind of input, and none of them is a finding yet:

SectionWhat to do with it
.claims.itemsThe PR's own claims. Each is a thing to CHECK, not a thing to believe — carry them into the correctness sweep.
.linked_tickets[].acceptance_criteriaWhat the change was meant to satisfy. Review against this, not only against the diff: a PR that is internally clean and misses an acceptance criterion is a finding.
.similar_prs.itemsPast PRs that rhyme with this one. Read the ones with the most probe hits — how they were structured, what reviewers caught, what broke afterward.
.bot_findingsHarvest, don't re-derive. What a bot already flagged is already raised; confirm or refute it with evidence rather than restating it in your own words and splitting the thread.
.threadsResolved vs unresolved state. A thread that got buried is not a thread that got addressed.

If a section comes back unavailable, say so in the review. Proceeding as though it were empty is how a gap in your coverage becomes a claim about the PR.

G3 — Run the lenses. The parallel analysis agents (R.3) and the convention audit (R.4). Feed them what G2 surfaced — the ticket acceptance criteria and the claims inventory in particular. Return here when they finish.

G4 — Sweep the dimensions (the exit gate). Work through skills/pr-review/DIMENSIONS.md, the eight-dimension completeness checklist next to this file, and name a result for every dimension, including an explicit "nothing here." Silence is not coverage: a dimension with no row is indistinguishable from one you never swept, and the reader cannot tell a deliberate "this is fine" from an accidental omission.

Four results are permitted — FINDING, CLEAR, N/A, NOT ASSESSED — and each carries its own evidence requirement. NOT ASSESSED is an honest, publishable answer and is strongly preferred over a CLEAR you cannot defend. Fill the table before ranking anything; written afterward it invites back-filling CLEAR into rows you never swept. The per-dimension probes, the exact requirements, and the table shape live in that file — follow it rather than re-deriving it here, and emit the full table in the review output.

R.3 Launch parallel analysis agents

Launch FOUR agents in parallel (single message, multiple Agent tool calls):

Agent 1: Convention compliance (the "standard" lens)

If STRUCTURAL_CONTEXT is non-empty, prepend to this agent's prompt:

## Structural Context
[Insert the insights and changed_clusters sections from STRUCTURAL_CONTEXT]
Explore agent (very thorough):
- Find 2-3 reference implementations for the same kind of change in this repo
- Compare the PR's patterns against them: type visibility, error handling, test
  conventions, logging, config
- Flag deviations from established patterns
- Check build hygiene: are dependency files tidy? (go.mod, package-lock.json, etc.)
  Do direct imports match direct dependency declarations?

Agent 2: Behavioral correctness (the "adversarial" lens)

If STRUCTURAL_CONTEXT is non-empty, prepend to this agent's prompt:

## Structural Context
[Insert full STRUCTURAL_CONTEXT block here]

Priority: trace call paths for high_blast_radius symbols first. Symbols with >20 callers have maximum blast radius — a correctness bug there affects the most code.
Explore agent (very thorough):
This is the most important agent. It traces execution paths, not patterns.

For each mutation path (write/delete/update) in the PR:
1. Trace the full path: trigger → handler → business logic → external API call
2. Identify all inputs to the path and ask: what if this input is truncated,
   empty, stale, or arrives out of order?
3. For event-driven code: enumerate every event type the system handles.
   For each event type, trace what happens. Then ask: what event sequences
   could produce bad state? (e.g., unverify then delete, create during cleanup)
4. For cleanup/deletion logic: what happens if the "source of truth" query
   returns a truncated or incomplete result? Does the code delete things it
   shouldn't?
5. For pagination: does the code page through all results or assume single-page?
   What is the consequence of truncation — missed work (recoverable) or
   data deletion (catastrophic)?
6. For error handling: trace every error path. Is the same error type handled
   consistently across all call sites? (e.g., if 401 is special-cased in one
   place, is it special-cased everywhere?)
7. For dry-run/safety modes: does dry-run cover ALL mutation paths, or only
   some? If the feature has 3 write paths and dry-run covers 2, that's a bug.

Rate severity based on CONSEQUENCE, not just code quality:
- Same pattern as existing code but worse consequence here = escalate
- Pre-existing bug but this PR makes it reachable from a new path = escalate
- Documented limitation with self-healing = note but don't necessarily block

Agent 3: Duplication and reuse

If STRUCTURAL_CONTEXT is non-empty, prepend to this agent's prompt:

## Structural Context
[Insert the insights and changed_clusters sections from STRUCTURAL_CONTEXT]
Explore agent (medium):
- For each new utility function in the PR, search the codebase for existing
  equivalents
- Check if any copied code exists behind internal/ in another module
- If code was copied, check for behavioral divergence from the original
  (different return values, different error handling, missing edge cases)
- Flag duplicated logic that should be extracted or imported

Agent 4: Infrastructure audit (only if PR contains .tf files)

Explore agent (medium):
- Read sibling IAC modules in the same directory
- grep -r for resource names the PR creates to check for collisions
- Verify module variable interfaces match actual upstream module signatures
- Check for hardcoded values that should be variables
- Check team/product/label tags for consistency with sibling modules
  (these affect alert routing, dashboards, and cost attribution)
- Verify secret injection method matches the module's expected pattern

Wait for all agents to complete.

Then earn every claim they made about something outside this repo. An agent that reports what an ORM tag, a query planner, a driver flag, or a wire format "does" is reporting its memory, and a confident wrong claim about a third party is the fastest way to lose a review's credibility:

<!-- @include-begin _shared/external-behavior-verification.md -->

Never assert third-party behavior from memory. ORM tag semantics, which plan the query planner will pick, which driver parameter actually takes effect, how a proto field lands on the wire, what a language feature does at an edge — these are the claims that feel most certain and are wrong most often, because what you remember is the common case and the finding always lives in the uncommon one.

Three things before such a claim leaves your hands:

  1. Name the layer that actually controls the behavior. The annotation, the driver, the connection string, the server-side default, and the dialect each get a vote, and only one of them decides. "There is a json: tag on the field" is not a mechanism; "the encoder at marshal.go:212 reads that tag and drops the zero value" is. A type name, an annotation, or a nearby symbol is a place to look, not a cause.
  2. Read that layer's official documentation — WebFetch or WebSearch the vendor's own page, not a blog post recounting it — and cite the URL plus the section. If you could not reach it, say the claim is inferred, and from what.
  3. Run it when running it is cheap. A five-line scratch program, one targeted test, an EXPLAIN ANALYZE, a throwaway container: each turns a remembered claim into a quoted output. Cite the output, not your memory of it.

This applies symmetrically. The same discipline that stops you shipping a wrong finding is what lets you confirm the author's claim — "the driver does coerce this, see the vendor doc §4.2" is worth as much as a defect, and it is the half reviewers skip because being right feels like finding nothing.

<!-- @include-end _shared/external-behavior-verification.md -->

R.4 Convention audit

If a language-specific standards skill exists (e.g., /go-standards), invoke it on all changed files.

Then run targeted grep audits on changed files:

CheckWhat to grep for
Hardcoded status codesNumeric literals in error-handling code
Wrong test contextManual context creation vs framework helpers
Constructor return typesConstructors returning concrete types vs interfaces
Dead test variablesVariables written but never asserted
Non-standard loggingImports of logging libraries that differ from repo convention
Untested code in mainFunctions with non-trivial logic in package main with no _test.go
Build hygieneDependency files tidy? Direct/indirect markers correct?

R.5 Synthesize findings

This is not just a merge — it's a structured synthesis that handles disagreement.

5.1 Group by location

Group all findings from all agents by file:line. When multiple agents flag the same location, note it.

5.2 Build the disagreement matrix

Where agents disagree on severity or whether something is an issue at all:

| Location | Convention | Behavioral | Duplication | IAC | Resolution |
|----------|-----------|------------|-------------|-----|------------|
| groups.go:38 | nit (pre-existing) | BLOCK (truncation → deletion) | — | — | ESCALATE: consequence is worse here |
| handler.go:95 | — | issue (event ordering gap) | divergence from original | — | Flag: needs human judgment |

5.3 Apply consequence-aware severity

For each finding, the final severity considers:

  • "This PR introduced it" vs "this code has the bug" — pre-existing bugs that the PR makes worse or newly reachable should still be flagged, but with explicit context about what's new vs inherited
  • Consequence escalation — same code pattern in two services can warrant different severity if the blast radius differs (e.g., missed backfill vs active data deletion)
  • Self-healing — a bug that self-heals on the next cron run is less severe than one that persists until manual intervention, but for customer-facing systems, "up to 1 hour of broken notifications" may still be unacceptable

5.4 Determine review body content

If STRUCTURAL_CONTEXT includes a non-trivial drift observation OR any high_blast_radius symbol:

  • Include a ## Structural Impact section in the review body:
    ## Structural Impact
    <2-3 sentences from the drift analysis>
    Changed clusters: [list]
    High blast-radius symbols: [list with caller counts]

Otherwise: body is verdict + blocking issues only. Do not add structural boilerplate to routine PRs.

5.5 Classify each finding

SeverityMeaning
BLOCKMust fix before merge. Data loss, security, or correctness bug.
FIXShould fix. Convention violation, missing test, inconsistency.
NITOptional. Minor improvement or a documentation gap.
FLAGNeeds human judgment. Agents disagree, or severity depends on product context.

R.6 Significance — triage the candidate set

Synthesis produced candidates. This phase decides which of them have earned a human's attention, and nothing reaches R.7 without passing through it.

Invoke the finding-triage skill (skills/finding-triage/SKILL.md) on the synthesized list. It applies two bars — TRUE (the code really behaves that way, and you can name the file:line where the mechanism lives) and MATTERS (acting on it changes this change, for a consumer that exists) — and returns exactly one disposition per candidate: confirmed, dropped, already-raised, already-addressed, or revised. Run it; do not re-derive its judgment here.

Record its dispositions in the review output, dropped rows included. Every candidate that went in gets a row coming out, and a dropped one carries the reason it was dropped and which bar it failed. This is not bookkeeping: a triage that quietly returns a shorter list is indistinguishable from a lazy pass that read three items and stopped, so the dropped rows are the evidence the gate ran. State both counts — candidates in, dispositions out.

Only confirmed and revised findings continue to R.7. The rest are reported as triaged, not as findings, and the already-raised rows in particular should credit whoever raised them rather than being restated in your own words.

R.7 Compile review

For each finding that cleared R.6:

  • File:line — specific location
  • What's wrong — the mechanism, stated plainly
  • Why — one line explaining the risk or convention
  • Severity — the internal rank from 5.5 (BLOCK / FIX / NIT / FLAG)
  • New vs inherited — did this PR introduce it, or is it pre-existing?

BLOCK / FIX / NIT / FLAG is how you rank; it is not what the author reads. The posted comment carries the reader-facing tag below — BLOCK → [Blocking], FIX and NIT → [Non-blocking], FLAG → [Question], and anything purely contextual → [Observation] — so the merge impact travels with the comment instead of staying in your head.

<!-- @include-begin _shared/comment-craft.md -->

Every finding carries a severity tag whose merge impact is explicit, so the author never has to guess whether a comment is a gate or a thought:

TagMeansMerge impact
[Blocking]Correctness, security, or data integrity.Blocks merge.
[Non-blocking]A real improvement, the author's call.Does not block.
[Question]A design clarification — you may be the one missing context.Does not block.
[Observation]Informational, for the next reader.No action implied.

Lead with the TL;DR in plain language, before any detail: what you found, whether it blocks, and what you would do. Burying the verdict under three paragraphs of trace makes the author read the whole comment to learn it was a nit.

Frame findings as questions rather than commands — "It looks like X — would it be worth considering Y?" rather than "You should do X." The imperative is only honest when you are certain, and you are certain less often than you feel; the question costs the same characters and leaves the author room to answer "no, because…" without it reading as defiance.

Every finding cites evidence: a file:line, another PR, a ticket, a doc URL, or the output of an experiment you actually ran. A finding with no citation is an opinion wearing a severity tag.

Never flag style, formatting, or naming. The formatter and the linter own those. A review that spends its first three comments there teaches the author to skim the rest of it.

Match the tone to the author. Someone senior in this code wants the mechanism and nothing around it; someone newer needs the why and a pointer to the pattern the codebase already uses. The finding is identical either way — only the scaffolding changes.

<!-- @include-end _shared/comment-craft.md -->

If STRUCTURAL_CONTEXT is non-empty, display the emergent diagram one final time alongside the compiled review for reference.

Lead with BLOCKs, then FLAGs (because these need human attention), then FIXes, then NITs.

If there are disagreements between agents, include a Disagreements section:

## Disagreements

D1. [Location] — Convention agent says [X]. Behavioral agent says [Y].
    Resolution depends on: [what human context is needed].

R.8 Render verdict

Conclude with exactly one of:

  • APPROVE — no blocking issues, nits only
  • REQUEST CHANGES — blocking or flagged issues exist but PR is salvageable
  • CLOSE PR — fundamental design problems require starting over

R.9 Present to user

Show the compiled review. Ask:

  • "Post this as a GitHub review? (approve/request-changes/comment)"
  • "Any items you want to remove or soften?"

If user approves, post via:

gh pr review $PR_NUM --request-changes --body "$(cat <<'EOF'
<review body>
EOF
)"

Or for inline comments, use the review comments API.


Respond Mode

You are the PR author addressing review comments.

S.1 Filter and triage comments

From the data in 1.2, identify:

  • Comments from reviewers (not the PR author)
  • Comments without replies (where in_reply_to_id is null)
  • Exclude comments already responded to

Already-addressed detection:

For each comment, check whether the referenced code was modified AFTER the comment was posted:

# Get the comment's creation date and the file it references
# Check if commits on this branch after that date modified the same file+region
git log --after="<comment_created_at>" --oneline -- <comment_path>

If the referenced lines were already changed in a subsequent commit, mark the comment as "likely addressed" and verify by reading the current code. Don't assume — confirm that the current code actually resolves the reviewer's concern.

S.2 Validate reviewer claims

Before implementing fixes, run a quick verification pass on non-trivial comments:

For each comment flagging a behavioral issue (not just style):
1. Read the code the reviewer is pointing at
2. Trace the execution path they describe — is their scenario actually possible?
3. If the reviewer's claim is wrong or already mitigated, note it
4. If the reviewer's claim is valid, note the fix approach

This prevents blindly implementing changes based on incorrect assumptions. Reviewers (human and AI) sometimes misread code flow or miss existing guards.

Apply the external-behavior discipline from R.3 here too, in the direction it is most often skipped: a reviewer's claim about what an ORM tag, a driver, or a wire format does needs the same mechanism-at-a-named-line standard before you rewrite code against it.

S.3 Organize and plan

Create a task list grouped by file/component:

CategoryDescription
Already addressedChanged in a prior commit — just needs a response
Quick winsTypos, formatting, naming, comments
Code changesRefactoring, helpers, pattern fixes
Behavioral fixesCorrectness issues, missing error paths, edge cases
Design questionsNeed discussion, not just a code change
TestingNew or modified tests requested

For each item, include a proposed action:

#1 groups.go:38 — Pagination risk [Behavioral fix]
   Proposed: Add pagination loop to ListVerifiedGroups using NextCursor
   Alternative: Document as known limitation matching existing pattern

#2 tenant-handler:97 — Unverification gap [Behavioral fix]
   Proposed: Skip Verified check for group.deleted.v1 events

#3 job.go:164 — Missing 401 on List [Quick win]
   Proposed: Add isUnauthorized check matching other call sites

#7 iac/main.tf:63 — Wrong team tag [Quick win]
   Proposed: Change to customer-platform to match event-reconciler

Present the plan with proposed actions. Ask: "Ready to proceed? Any you want to change, skip, or handle manually?"

S.4 Implement fixes

Work through the task list:

  • Start with already-addressed items (just confirm, no code change needed)
  • Then quick wins (reduces noise for re-review)
  • Then behavioral fixes and code changes
  • Group related changes for atomic commits
  • Read files before editing
  • Only fix what was requested — no scope creep
  • Stage specific files (never git add -A)

S.5 Quality checks

Launch TWO agents in parallel:

Agent 1: Convention review

Review the changes just made:
- Do fixes follow repo patterns?
- Any new violations introduced while fixing?

Agent 2: Behavioral verification

For any behavioral fixes made:
- Does the fix actually address the traced execution path?
- Did the fix introduce new edge cases?
- Is the fix tested?

Fix any issues found.

S.6 Commit

git add <specific files>
git commit -m "refactor: address PR review comments

- <list of changes>

Addresses review comments from @reviewer."

S.7 Generate responses

Every response MUST include a GitHub permalink to the specific line(s) changed. No bare commit hashes. The reviewer should be able to click one link and see exactly what changed.

Step 1: Collect the ingredients.

# Full SHA for stable permalinks
SHA=$(git rev-parse HEAD)

# Owner/repo from the PR (already fetched in Phase 1)
# e.g., "myorg/myrepo"
OWNER_REPO=$(gh pr view $PR_NUM --json headRepository --jq '.headRepository.owner.login + "/" + .headRepository.name')

Step 2: For each comment, find the lines that address it.

Each review comment from Phase 1 has a path field (the file the reviewer commented on). Find the relevant lines in the current HEAD:

# Get the PR's base branch
BASE=$(gh pr view $PR_NUM --json baseRefName --jq '.baseRefName')

# Show changed line numbers in the file across the entire PR
git diff "origin/$BASE"...HEAD --unified=0 -- <comment_path> | grep '^@@' | head -10
# Output like: @@ -38,1 +38,15 @@ → means lines 38-52 in the new file

Read the @@ hunks to get the line range. Pick the hunk most relevant to the reviewer's comment, then build the permalink:

https://github.com/{OWNER_REPO}/blob/{SHA}/{path}#L{start}-L{end}

If the fix spans multiple hunks in the same file, link the most relevant one. If the fix is in a DIFFERENT file than the comment, link the new file.

Step 3: If you cannot determine the line range, fall back to file-level link (#L1 omitted), but this should be rare. Never fall back to a bare commit hash.

Response templates. Three shapes, not six. Every reply to a review thread is one of exactly three things — the code changed, the code is not changing, or you asked something and here is the answer — and a reply that fits none of them is a reply you have not finished thinking about. Near-identical templates that differ only in how much prose follows invite picking by feel; one shape per outcome holds under load. Optional lines stay optional: drop the explanation when the diff is self-evident.

1 — Changed. One location or many; the explanation line is optional.

Fixed — [{path}#L{start}-L{end}](https://github.com/{owner}/{repo}/blob/{sha}/{path}#L{start}-L{end})

[Optional: one line on what changed and why, when the diff alone doesn't say it]
Fixed in {sha_short}:
- [{path1}#L{line}](permalink1) — [what changed here]
- [{path2}#L{line}](permalink2) — [what changed here]

2 — Not changing. Covers a deliberate decline and a pre-existing issue; the only difference is whether a ticket carries it forward, so say which.

Keeping as-is — [reasoning].

Current code: [{path}#L{line}](permalink)
[Evidence or reference supporting the decision]
This is pre-existing behavior — see [{path}#L{line}](permalink). Filed {ticket} to address separately.
[Why fixing it here would expand scope beyond this PR]

3 — Answering. A design question or a clarification, where the deliverable is reasoning rather than a diff.

[Thoughtful explanation with context]

See the implementation: [{path}#L{start}-L{end}](permalink)

Technical rationale: [...]
Trade-offs considered: [...]

Refusal condition — a template is a gate, not a mould. If you cannot build the permalink for shapes 1 or 2, the reply is not posted. Fall back to the file-level link per Step 3; if even the file is uncertain, you have not located the fix you are claiming to have made, and saying "fixed" is an assertion you cannot back. Surface it as unresolved instead and say what you could not find. A reply whose evidence link is wrong is worse than no reply: it spends the author's trust and costs them a round trip to discover the claim was empty.

S.8 Present and post

Show all responses grouped by thread. Include:

  • Comment ID (for posting via gh)
  • File and line context
  • Proposed response text with permalinks already rendered

HARD GATE: Before presenting responses to the user, scan every response body for github.com/.../blob/. If ANY response is missing a permalink, STOP — go back to Step 2 and find the lines. Do not present responses without permalinks. Do not substitute bare commit hashes as a workaround.

Ask: "Ready to post these responses? Any you want to modify?"

If approved:

# Push first (permalinks need the SHA to be on the remote)
git push

# Post inline replies
gh api repos/{owner}/{repo}/pulls/{pr_num}/comments \
  -X POST \
  -f body="Fixed — see [path#L42](https://github.com/owner/repo/blob/abc1234/path#L42)" \
  -F in_reply_to=COMMENT_ID

S.9 Resolve threads + re-request review

After posting (S.8), resolve any thread whose reply just confirmed a fix, and re-request review if the PR needs another look. This is pure gh api (REST for re-request, GraphQL for thread resolution — GitHub's REST API has no resolve-thread endpoint) — no GitHub App/MCP connector needed. Full detail (ID-mapping query, the resolve-gate rule, the mutation, the re-request command) lives in skills/pr-review-kit/ENGAGE.md §9.6 — follow it as written rather than re-deriving; the short version: only resolve threads you've both replied to AND confirmed the code addresses, and only re-request review with explicit user authorization (same posting gate as S.8).

S.10 Ticket update (optional)

If a Linear ticket is linked:

PR review comments addressed:
- Fixed N/M comments in <commit>
- K design questions answered
- Quality checks: passed

Ready for re-review.

Error Handling

PR not found: List recent PRs via gh pr list, ask user to specify.

Linear ticket not found: Skip Linear integration, continue with PR-only workflow.

Quality checks fail: Show findings, ask whether to fix and retry or proceed. Never auto-push with failures.

Git push fails: Check if branch is behind remote. Offer to pull and retry. Don't force-push without confirmation.

No unaddressed comments (respond mode): Report "All comments addressed" and exit.

Empty diff (review mode): Report "No changes to review" and exit.


Edge Cases

Comment already has reply: Skip in respond mode. Acknowledge in summary.

Conflicting reviewer comments: Flag for user attention. Don't auto-implement contradictions.

Large PR (>15 files or >500 lines): Suggest reviewing in chunks by commit or component. Offer to focus on the riskiest files first.

Large number of comments (>15): Offer to batch in respond mode. Tackle quick wins first.

PR from a fork: Adjust gh api paths. Flag if push access may be limited.

Pre-existing bugs surfaced by review: Distinguish clearly. If the PR didn't introduce the bug but makes it worse or newly reachable, flag it as "inherited, escalated." If the bug is completely pre-existing and unrelated to the PR's changes, note it but don't block.


Example Usages

# Review someone else's PR
/pr-review 34545

# Respond to comments on your PR (auto-detect from branch)
/pr-review

# Explicit modes
/pr-review --review 34545
/pr-review --respond 34545

# From Linear ticket
/pr-review ENG-456

Integration Points

GitHub: gh pr view, gh pr diff, gh api (comments, reviews), gh pr review

Linear (optional): Find PR link, fetch context, post update

Git: Branch detection, clean commits, push

Analysis Agents: Convention compliance, behavioral correctness, duplication, IAC audit

Standards Skills: /go-standards, /python-standards, etc. — invoked for convention audits


Design Rationale

This skill uses a multi-lens review model inspired by adversarial review systems that run "good cop" and "bad cop" passes independently, then synthesize.

Why multiple agents instead of one pass? A single reviewer tends to anchor on either convention or correctness, rarely both. Convention-focused reviews catch style issues but miss behavioral edge cases (event ordering, pagination truncation, incomplete dry-run coverage). Behavioral reviews catch correctness bugs but miss style violations. Running both in parallel and synthesizing produces better coverage than either alone.

Why the disagreement matrix? When two agents flag the same location for different reasons, that's the highest-value signal. When they disagree on severity, that reveals a judgment call that needs human input. Surfacing disagreements explicitly (rather than picking a winner) gives the reviewer the information they need to decide.

Why consequence-aware severity? The same code pattern can be a nit in one service and a blocker in another. A single-page list call that truncates results is a nit when the consequence is "missed backfill that self-heals." It's a blocker when the consequence is "actively deletes customer data for orgs on page 2+." The behavioral agent is instructed to rate by consequence, not pattern.

Why "new vs inherited"? Agent reviewers struggle to distinguish "this code has a bug" from "this PR introduced a bug." Both are factually accurate observations, but they require different responses. A pre-existing bug that's unrelated to the PR is informational. A pre-existing bug that the PR makes worse or newly reachable is actionable. Labeling each finding as new or inherited gives the human reviewer the context to make that call.


Notes

  • Review mode does NOT write code — it produces a review document
  • Respond mode DOES write code — it implements fixes and commits
  • The behavioral correctness agent is the most important addition — it traces execution paths rather than matching patterns, catching edge cases that static analysis misses
  • Always confirm with user before posting reviews or pushing code
  • The convention compliance check reuses the same pattern-discovery approach from /feature-impl Phase 0, repurposed as a review lens

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.