Report Format
pr-review generates a structured Markdown report saved to the current directory. The filename includes the PR number when detected via the gh CLI. Every section is written for both developers and non-technical stakeholders.
Output filename
| Condition | Filename |
|---|---|
| PR detected (e.g. #42) | pr-42-review.md |
| No PR detected | pr-review-review.md (from config) |
Custom via --output | Your specified filename |
Report sections
| Section | What it contains | Format |
|---|---|---|
| 📋 Summary | Plain-English overview of what the PR changes, who it affects, and overall quality | Paragraph |
| 🗂️ At a Glance | One-row-per-finding summary table across all categories with severity and "must fix?" flag | Table |
| 🚨 Critical Issues | Bugs and breaking changes that must be fixed before merge, with real-world user impact | Table |
| 🔒 Security | Auth bypasses, secret exposure, injection risks — with who is affected and a fix | Table |
| ⚡ Performance | Slow queries, memory leaks, blocking I/O — with user-facing impact described plainly | Table |
| 🛠️ Code Quality | Non-blocking improvements: error handling, naming, duplication, anti-patterns | Table |
| 🧪 Test Coverage | Missing tests, untested edge cases, and suggested test cases to add | Table |
| 💡 Suggestions | Optional nice-to-haves with effort estimates (🟢 Low / 🟡 Medium / 🔴 High) | Table + bullets |
| ✅ Final Verdict | APPROVE / REQUEST CHANGES / NEEDS DISCUSSION with plain-English justification and blocking item list | Paragraph + bullets |
Severity levels
| Indicator | Meaning | Action required? |
|---|---|---|
| 🔴 Critical / High | Data loss, security breach, or broken functionality likely | Must fix before merge |
| 🟡 Medium | Noticeable user impact or elevated risk under certain conditions | Strongly recommended |
| 🟢 Low | Minor improvement with no immediate risk | Nice to have |
Sample report
A condensed example showing all sections. See the full Example Report for a comprehensive real-world sample you can download.
pr-42-review.md
# PR Review: `feature/auth` → `main`
**PR:** #42 — feature/auth → main
**Date:** 2026-04-08
**Provider:** claude (claude-sonnet-4-6)
**Files reviewed:** 3 files, 142 insertions, 28 deletions
---
## 📋 Summary
This PR introduces JWT-based authentication middleware. The overall structure is solid, but there
are two critical security issues that must be resolved before this is safe to merge. Written for
both developers and non-technical reviewers — no coding knowledge required to understand the risks.
---
## 🗂️ At a Glance
| # | Category | Severity | What's the problem? | Must fix before release? |
|---|----------|----------|---------------------|--------------------------|
| 1 | 🚨 Critical | 🔴 High | JWT token expiry is never checked — expired tokens stay valid forever | Yes |
| 2 | 🔒 Security | 🔴 High | JWT secret falls back to empty string if env var is missing | Yes |
| 3 | 🛠️ Quality | 🟢 Low | Magic number 3600 should be a named constant | No |
---
## 🚨 Critical Issues
> These problems **must be fixed before this change goes live**.
| Severity | What's the problem? | Real-world impact on users | File / Location | What the developer should do |
|----------|---------------------|----------------------------|-----------------|------------------------------|
| 🔴 Critical | JWT token expiry (`exp` claim) is decoded but never validated | A logged-out or deactivated user's token remains valid indefinitely — they can keep accessing protected pages | `src/middleware/auth.js:42` | Add `if (Date.now() >= payload.exp * 1000) throw new TokenExpiredError()` after decoding |
---
## 🔒 Security
> Security issues can expose user data or allow unauthorised access.
| Severity | What's the risk? | Who is affected? | File / Location | Recommended fix |
|----------|-----------------|-----------------|-----------------|-----------------|
| 🔴 High | JWT secret falls back to `""` (empty string) when `JWT_SECRET` env var is missing — tokens can be forged by anyone | All authenticated users | `src/config/jwt.js:8` | Throw at startup: `if (!process.env.JWT_SECRET) throw new Error("JWT_SECRET is required")` |
---
## 🛠️ Code Quality & Improvements
| Area | What's the issue? | Why it matters | File / Location | Recommended improvement |
|------|------------------|----------------|-----------------|-------------------------|
| Magic numbers | The 3600-second expiry is hardcoded inline | If the value needs to change it must be updated in multiple places | `src/config/jwt.js:14` | Extract to `const TOKEN_EXPIRY_SECONDS = 3600` at the top of the file |
---
## 🧪 Test Coverage
| What's not being tested? | Why it matters | Possible consequence if it breaks | Suggested test to add |
|--------------------------|----------------|-----------------------------------|-----------------------|
| Expired token rejection | Tokens must be refused after expiry | Expired sessions remain active — security gap | Test that a token with a past `exp` returns `401 Unauthorized` |
---
## 💡 Suggestions & Nice-to-Haves
| # | Suggestion | Benefit | Effort |
|---|-----------|---------|--------|
| 1 | Add refresh token support | Users won't be logged out mid-session unexpectedly | 🟡 Medium |
| 2 | Log auth failures (wrong token, expired) to your observability tool | Helps detect brute-force attempts early | 🟢 Low |
---
## ✅ Final Verdict
**REQUEST CHANGES**
Two security issues block this merge: expired tokens are never rejected, and the JWT secret can
silently fall back to an empty string. Both are straightforward fixes. Once resolved and covered
by tests, this PR is ready to ship.
**Blocking items from Glance before merge:**
- Fix #1 — Validate the `exp` claim after decoding (row 1)
- Fix #2 — Throw on missing `JWT_SECRET` at startup (row 2)