Skip to content

Gryz Skill Library

Code review

Review a code diff or pull request for minimal blast radius, scope discipline, security-sensitive domains (auth, billing, admin, secrets), and test coverage on new logic. Use when the user asks to "review this PR/diff", "does this change look safe to merge", or pastes a patch for feedback before merging. Do NOT use for reviewing prose, product specs, or non-code documents — use editorial-review for those.

v1 · authored by Gryz · engineering

Use it from any MCP-connected agent

Connect Gryz MCP, then call list_skills and get_skill({ name: "code-review" }). Everyone can use Library skills; Pro can fork and edit this one, and Pro Plus can create your own from scratch.

# Code review Review a code diff or pull request the way a careful senior engineer would: scope discipline first, security-sensitive domains second, tests third. ## When to use - "Review this PR/diff", "does this change look safe to merge", or the user pastes a patch or a link to a diff for feedback before merging. ## Do NOT use for - Reviewing prose, product specs, or design docs — use editorial-review. - Writing the code yourself unprompted — review what's there; don't rewrite it unless asked. ## Review checklist, in priority order 1. **Blast radius.** Does the diff change only what the stated task needs? Flag any unrelated rename, reformat, refactor, or "while I'm here" cleanup — even a good one — as out of scope for this change. 2. **Security-sensitive domains.** Extra scrutiny on anything touching auth, session/token handling, billing/webhooks, admin routes, secrets/env vars, or a database migration: - No hardcoded credentials, ever. - No secret exposed to client-side code. - Auth/ownership checks present on every new read or write path — not assumed from a caller-supplied id. - Webhook handlers verify signatures and are idempotent. - A migration only adds a new file — it never edits one already applied. 3. **Correctness.** Does the change actually do what the PR description says? Look for off-by-one errors, unhandled error branches, and inconsistent state left behind if a step fails partway through. 4. **Test coverage.** Does new critical logic (pure functions, tier/limit math, validation, anything auth/billing touches) have a test? Flag its absence as a real finding, not a nice-to-have. 5. **Conventions.** Does the diff match the surrounding file's existing style (naming, indentation, import order) rather than imposing a new one? ## How to respond Group findings by severity — critical (blocks merge: security, correctness), then scope/blast-radius, then minor/style — and cite the specific line or hunk for each. Don't restate the whole diff back; point at what's wrong and why. ## Examples **Finding:** "Critical — `app/api/admin/users/route.ts` line 42: this route reads `req.headers.get('x-user-id')` to decide the target user instead of the authenticated session. Any caller can act as any other user by setting that header." **Finding:** "Scope — this diff also reformats `lib/utils.ts` (200 lines changed) unrelated to the stated bug fix. Recommend splitting into a separate PR so the real fix is reviewable."