| name | pr-review |
| description | Review the current branch's diff (or the latest commit on main) against bun-mono's house style, TypeScript conventions, test discipline, and security baselines. Use when the user is about to push, open a PR, or asks "review my branch", "review before push", or "code review this". |
PR review
Reviews not-yet-pushed work against the project's actual standards, not generic CR heuristics. Rules come from [[house-style]] and the checks below — anything mechanical (oxlint, oxfmt, tsgo) is delegated to bun check; this skill catches what those tools can't.
Workflow
-
Pick the diff range — auto-detect, don't ask:
- On a branch other than
main → git diff main...HEAD (use the three-dot range so renames + merges are handled).
- On
main → git show HEAD (the latest commit).
- If the user passed a ref (e.g.
/pr-review HEAD~3), use git diff <ref>...HEAD.
- Also include uncommitted changes if any:
git diff + git diff --cached. Flag them as "uncommitted" in the report — they won't be in the push.
-
Run bun check before reviewing. If it fails, stop and report the failure as a must-fix; reviewing code that doesn't compile / lint / test is wasted work. The Stop hook already runs format + types after each turn, but bun check also runs tests and builds, which the hook skips.
-
Walk the diff applying Review dimensions. For each finding capture: file:line, the rule (quote the source), severity (must-fix / should-fix / nit).
-
Produce one categorized report in the format below.
-
Offer fixes — ask: "Apply the must-fix items? (y / pick / n)". On y, apply them sequentially, showing each diff and pausing for confirmation. Don't auto-fix should-fix or nits; those are judgment calls the user owns.
Review dimensions
House-style (see SKILL.md)
- No barrel
index.ts: every import targets the concrete duck file.
- File size: > 400 lines is a split candidate unless it's a single cohesive algorithm (e.g.
engine.ts).
- One concern per file: a
.tsx shouldn't also export hooks or generic utils — those move to their own duck.
- Duck/feature layout: new files land under a feature directory; no type-bucketed
components/ or hooks/ directories.
- Blank-line groups in function bodies: each phase (extract → compute → guard → emit) separated by a blank; a
const and its immediate guard packed; parallel facts of the same shape packed; every useState / useMemo / useCallback starts its own phase with a blank above it.
- JSX siblings: distinct siblings get blanks;
.map() output and list-shaped same-tag siblings (consecutive <DropdownMenuItem>) stay packed; label+value composing one visual line stays packed.
TypeScript
- No
enum, no namespace (banned by erasableSyntaxOnly). Use string-literal unions.
- No
any; reach for unknown and narrow, or fix the type at the boundary.
type over interface unless declaration merging is required.readonly Array<T> / ReadonlySet<T> on values returned from pure functions.- Branded primitives where invariants exist (
type Seat = 0 | 1 | 2 | 3, not number).
- New library packages with hand-written public surface → confirm
isolatedDeclarations: true. Packages wrapping a schema/DSL builder (drizzle, better-auth, oRPC, t3-env, cva, arktype) are exempt — see the caveat table.
- New duck file →
package.json#exports entry was added (via bun turbo gen export).
Tests
- No tests that restate the type signature or the implementation. If TypeScript already rejects it, don't write a test for it.
- Pure-function tests preferred over DOM render assertions.
vitest-playwright only when the test genuinely needs a real browser.
- No mocks of internal collaborators within the same package — if you need to, the seam is wrong.
- Test names describe one behavior, not "works correctly".
- New behavior in a duck → matching test in
<duck>.test.ts.
Security & correctness
- No secrets / API keys / tokens /
.env contents in the diff. Grep the patch for high-entropy strings.
- HTTP / handler input validated via arktype at the boundary; don't re-validate downstream.
- DB access via Drizzle's parameterized API — no string-concatenated SQL.
dangerouslySetInnerHTML only with explicitly sanitized input; flag every instance.process.env / Bun.env access goes through @bun-mono/env, not raw.- No new
console.log of request bodies, tokens, or PII.
- Auth checks present on protected routes / oRPC procedures.
Output format
# PR review: <branch> → main (N files, +X / −Y)
`bun check`: ✅ pass | ❌ fail — <one-line summary, link to failing command>
## Must-fix
- **`packages/royalty/src/seat/seat-utils.ts:42`** — file is 480 lines mixing 3 hooks + 2 pure utils. House-style: "one concern per file ... should not also export hooks or generic utils."
- **`apps/server/src/auth.ts:18`** — `console.log(token)` leaks the session token on every request.
## Should-fix
- **`packages/api/src/router.ts:55`** — `payload: any`. Switch to `unknown` + arktype validator at the boundary.
## Nits
- **`packages/royalty/src/card/card-button.tsx:12-14`** — three `useCallback`s packed without blanks. House-style: "each hook is its own statement and starts its own phase."
## Notes
- <test gaps, follow-ups, anything not a finding but worth flagging>
After the report: "Apply the must-fix items? (y / pick / n)" — pick lets the user select a subset by number.
What this skill does not do
- Doesn't run
/review (the built-in cloud reviewer) — that's /ultrareview if the user wants multi-agent cloud review.
- Doesn't push, open PRs, or comment on GitHub — review only.
- Doesn't re-check things
oxlint / oxfmt / tsgo already enforced; trust bun check.
