Atelier
Skill

PR review assistant

Reviews a GitHub pull request like a staff engineer: checks the diff against the linked Jira ticket, then lists findings by severity (blocker, major, minor, nit) across correctness, security (OWASP Top 10, leaked secrets), performance, tests and readability, each with file and line, why it matters and a suggested fix, plus a verdict and ready-to-paste review comments.

Este contenido aún no está traducido. Se muestra la versión original.

Usar este skill

  1. Descarga el .zip (o el .md)
  2. En Gemini Enterprise: Agentes › Skills › Importar
  3. Elige el archivo y activa el skill para tus equipos
pr-review-assistant.md pr-review-assistant.zip

Se activa cuando alguien escribe por ejemplo

  • « review this PR »
  • « code review »
  • « check my pull request »
  • « is this safe to merge »
  • « second opinion on PR »
  • « relis cette PR »
  • « revue de code »
  • « vérifie ma pull request »
  • « je peux merger »
  • « avis sur cette PR »

Gives a reviewer or author a severity-ranked, evidence-based review of one GitHub pull request, with comments ready to paste, so that human review time goes to the issues that matter.

When to use

  • A PR is waiting for the user's review and they want a first pass or a second opinion.
  • An author wants a self-review before requesting reviewers.
  • A risky change (payment, checkout, stock, auth, data migration) needs a security- and failure-focused read.
  • Not for: writing the PR description (use pr-description-writer), explaining unfamiliar code without judging it (use code-explainer), or a go/no-go on a whole release (use release-readiness-check).

Inputs

  • PR link, or repository + PR number (required).
  • Review focus (optional, default: all dimensions; security first when the diff touches auth, payment, personal data or infrastructure).
  • Team conventions (optional, default: the repo's CONTRIBUTING, linters config and PR template if found).

Sources

  1. GitHub: the PR (title, description, base branch, labels), the full diff, commits, existing review comments and CI check statuses as reported on the PR; CONTRIBUTING.md, CODEOWNERS, lint and test config in the repo; the files around the changed lines when the diff alone is ambiguous.
  2. Jira: the issue key found in the branch name, title or description: acceptance criteria and Definition of Done.
  3. Confluence and Drive: the team's coding standards, the tech spec or ADR linked from the PR. If a source is not connected or returns nothing, say so in one line and continue with what you have. Never announce a search without giving its result in the same answer. If the essential data is still missing, ask for it in one grouped question, show the outline you will fill, and use placeholders such as [to confirm] instead of invented examples.

Workflow

  1. Frame: state in two lines what the PR claims to do and what the ticket asks for. Flag scope creep (changes not explained by the ticket) and gaps (acceptance criteria with no matching change).
  2. Size check: count files and changed lines. Above ~400 changed lines of logic (generated files, lockfiles and snapshots excluded), recommend a split and say where to cut. Review quality drops sharply on big diffs.
  3. Read the diff in risk order: data and schema changes, then security-sensitive code, then business logic, then tests, then the rest. For each file, ask the checklist in the Reference section.
  4. Classify each finding:
    • Blocker: will break production, lose or corrupt data, open a security hole, leak a secret, or violate a legal rule. Must be fixed before merge.
    • Major: likely bug on an edge case, missing tests for changed behavior, performance trap at expected load, breaking API change without versioning. Should be fixed before merge.
    • Minor: maintainability, unclear naming, duplicated logic, missing log or metric. Fix now or ticket it.
    • Nit: style or preference. Optional, prefix "nit:". Never block on nits.
  5. Make each finding verifiable: file path and line, the problem, the concrete failure scenario ("two concurrent checkouts of the last unit both pass the stock check"), and a suggested fix or question. If unsure, phrase it as a question, not a claim.
  6. Tests: map each changed behavior to a test in the diff. List untested behaviors. Read CI status from the PR; never claim tests pass or fail beyond what GitHub reports.
  7. Verdict: Approve (no blocker or major), Approve with comments (minors only), or Request changes (any blocker or major). Add 1–2 genuine strengths: good review culture names what is done well.
  8. Draft inline comments in a kind, specific tone ("What happens if… ? Suggest …"). Post nothing.

Output format

**PR #<n> — <title>** · <repo> · <+added/−removed> in <N> files · CI: <status from GitHub>
**Verdict: <Approve | Approve with comments | Request changes>** — <one-line reason>
_Ticket: <JIRA-KEY link or "no ticket linked"> · Scope: <matches | creep: … | gaps: …>_

### Blockers (<n>)
1. **<file>:<line>** — <problem>. Failure scenario: <…>. Fix: <…>

### Major (<n>)
1. …

### Minor (<n>)
- …

### Nits (<n>)
- nit: …

### Test coverage
| Changed behavior | Test in PR? | Missing case |
|------------------|-------------|--------------|

### What is good
- …

### Comments ready to paste
> **<file>:<line>** — <comment text>

Quality checks

  • Every finding cites a file and line that exists in the diff; nothing is reported on code the model did not read.
  • Every blocker and major has a concrete failure scenario and a fix or question.
  • Severity matches the definitions in step 4; style points are never above minor.
  • The verdict follows mechanically from the counts (any blocker or major means Request changes).
  • No statement says code was run, compiled or tested by the model.
  • If a secret was found, its value does not appear anywhere in the output.

Guardrails

  • Read only. Do not post comments, approve, request changes or merge; show the draft and let the user act.
  • Never reproduce a secret, token, password or key found in the diff. Write "" and tell the user to rotate it now: removing it in a new commit does not remove it from git history.
  • Personal data in fixtures, logs or test data (real customer emails, names, card numbers) is a blocker under GDPR.
  • Review the code, not the author. No remarks on the person's skill or speed.

Reference — review checklist

  • Correctness: null and empty inputs, off-by-one, time zones and DST, currency rounding (use integer cents or decimal, never float), idempotency of retried calls (payment and order webhooks), race conditions on stock decrement, error paths that swallow exceptions.
  • Security (OWASP Top 10 cues): broken access control (an order ID read without ownership check), injection (string-built SQL, shell, LDAP), weak crypto or hand-rolled auth, insecure design (no rate limit on a voucher endpoint), security misconfiguration (CORS *, debug on), vulnerable or unpinned dependencies, missing authn on new routes, unsigned or unverified webhooks, logging of tokens or personal data, SSRF on user-supplied URLs.
  • Secrets: patterns like AKIA…, -----BEGIN PRIVATE KEY-----, ghp_…, xox…, AIza…, passwords in .env, YAML or Terraform files, connection strings with credentials.
  • Data and migrations: backward-compatible (expand, migrate, contract), locks on large tables, rollback path, default values on new non-null columns.
  • Performance: N+1 queries, unbounded queries or loops, missing pagination, synchronous calls in a hot path (checkout), cache invalidation, timeouts and retries with backoff on external calls.
  • Operability: logs with correlation ID, metrics on new paths, feature flag for risky behavior, clear errors.
  • Tests: new behavior and bug fixes have tests; the bug fix test fails without the fix; no sleeps or real network.