requesting-code-review — independently scanned and version-tracked by SaferSkills.
SaferSkills independently audited requesting-code-review (Agent Skill) and scored it 100/100 (green). The audit ran 55 deterministic rules across Security, Supply Chain, Maintenance, Transparency, and Community; it found 0 high-severity and 0 lower-severity findings. The full rule-by-rule trace and per-finding evidence are below. Free, methodology-open.
Findings & checks · 0 flagged
Every scanned point with the score it earned and what moved between them.
First recorded scan — no prior version to compare against.
The primary manifest — the file an agent reads to learn what this artifact does.
Prepare your code for review before asking others to look at it. A well-prepared review request gets faster, higher-quality feedback and respects the reviewer's time.
Core principle: The author's job is to make the reviewer's job easy. Every minute you spend preparing saves ten minutes of back-and-forth.
Violating the letter of this process is violating the spirit of code review.
Always use for:
Lighter preparation acceptable for:
Never skip:
NO REVIEW REQUEST WITHOUT PASSING TESTS AND SELF-REVIEWIf tests fail, fix them. If you haven't read your own diff, read it. Requesting review on broken or unreviewed code wastes everyone's time.
| Phase | Purpose | Time |
|---|---|---|
| 1. Pre-Flight Checks | Validate code is review-ready | ~2 min |
| 2. Self-Review | Catch issues before the reviewer does | ~5-15 min |
| 3. Change Summary | Explain what, why, and impact | ~5 min |
| 4. Test Evidence | Prove it works | ~2 min |
| 5. Review Focus Areas | Guide the reviewer's attention | ~3 min |
| 6. Review Request Assembly | Format the PR description | ~5 min |
Run before anything else. All must pass.
# Tests passing
npm test
# Expected: all green, zero failures
# TypeScript compilation
npx tsc --noEmit
# Expected: no errors
# Linting
npx eslint .
# Expected: no errors (warnings acceptable)
# Formatting
npx prettier --check .
# Expected: all files formattedIf the project has a validate script, use it:
npm run validate
# Runs all checks in one command# No TODO/FIXME in your changes
git diff --cached | grep -n "TODO\|FIXME\|HACK\|XXX"
# Expected: no matches (or documented exceptions)
# No unintended files
git diff --cached --name-only
# Review: every file should be intentional
# No debug artifacts
git diff --cached | grep -n "console\.log\|debugger\|\.only("
# Expected: no matchesIf ANY check fails → fix before proceeding. No exceptions.
Read your own diff as if you are the reviewer.
# View the full diff
git diff main...HEAD
# Or for staged changes:
git diff --cached- [ ] Every changed file is intentional (no accidental modifications)
- [ ] Variable and function names are clear without context
- [ ] No hardcoded values that should be constants or config
- [ ] Error cases handled (not just happy path)
- [ ] Edge cases considered (null, empty, boundary values)
- [ ] No copy-pasted code that should be extracted
- [ ] Comments explain "why", not "what"
- [ ] No leftover debugging code| Issue | Detection | Fix | |||
|---|---|---|---|---|---|
| Forgotten console.log | grep -rn "console.log" src/ | Remove or replace with proper logging | |||
| Hardcoded secrets | `grep -rn "password\ | secret\ | token\ | apiKey" src/` | Move to environment variables |
| Missing error handling | Look for unhandled promises, bare try/catch | Add specific error handling | |||
| Overly large diff | git diff --stat shows 500+ lines | Split into smaller PRs |
Answer three questions:
# File-level summary
git diff main...HEAD --stat
# Logical grouping of changes
git log main..HEAD --onelineCategorize changes:
Link to the motivation:
# Find direct dependents of changed files
git diff main...HEAD --name-only | while read file; do
echo "=== $file ==="
grep -rn "from.*$(basename $file .ts)" src/ --include="*.ts" | head -5
doneAssess impact radius:
Show, don't tell. Include actual output.
# Run tests with verbose output
npm test -- --reporter=verbose 2>&1 | tail -20
# Example output to include:
# ✓ should create user with valid email (3ms)
# ✓ should reject duplicate email (2ms)
# ✓ should handle network timeout (15ms)
# Tests: 47 passed, 0 failed# Generate coverage report
npm run test:coverage 2>&1 | grep -A 5 "Coverage summary"
# Example output:
# Statements : 92.3% (+1.2%)
# Branches : 87.5% (+3.0%)
# Functions : 95.0% (+0.5%)
# Lines : 91.8% (+1.1%)If coverage decreased, explain why:
List new test cases with descriptions:
New tests:
- "should validate email format before creating user"
- "should return 409 when email already exists"
- "should retry on transient network failure"
- "should timeout after 5 seconds"Guide the reviewer to what matters most.
| Category | What to Flag | Example |
|---|---|---|
| Complex logic | Algorithms, state machines, tricky conditions | "The retry logic in fetchWithBackoff handles 5 failure modes" |
| Security-sensitive | Auth, input validation, data exposure | "New endpoint requires auth — please verify middleware chain" |
| Breaking changes | API changes, schema migrations, config changes | "Response format changed from array to paginated object" |
| Uncertainty | Code you're not confident about | "Not sure if this caching strategy handles concurrent writes correctly" |
| Trade-offs | Deliberate compromises | "Chose N+1 query for readability; volume is < 100 items" |
### Review Focus Areas
1. **[Complex]** `src/services/retry.ts:25-60` — Exponential backoff
with jitter. Please verify the max delay calculation.
2. **[Security]** `src/middleware/auth.ts:15` — Added rate limiting.
Check if the window size is appropriate.
3. **[Breaking]** `src/api/users.ts:42` — Changed response from
`User[]` to `{ data: User[], pagination: Pagination }`.
All consumers need to update.
4. **[Uncertain]** `src/cache/store.ts:78` — Cache invalidation
on concurrent writes. Would appreciate a second opinion.Combine all phases into a structured PR description.
## Summary
[1-3 sentences: what this PR does and why]
Closes #[issue-number]
## Changes
- [Categorized list from Phase 3]
## Test Evidence
- All tests passing: [X] passed, 0 failed
- Coverage: [X]% (+/-Y%)
- New tests: [count] added
## Review Focus Areas
1. **[Category]** `file:line` — [Description]
2. **[Category]** `file:line` — [Description]
## Impact
- **Breaking changes:** [Yes/No — details if yes]
- **Dependencies affected:** [list or "none"]
- **Migration needed:** [Yes/No — steps if yes]
## Checklist
- [ ] Tests passing
- [ ] Lint clean
- [ ] No TODO/FIXME in diff
- [ ] Self-reviewed
- [ ] Documentation updated (if needed)<type>(<scope>): <description>
Examples:
feat(auth): add JWT refresh token rotation
fix(api): handle timeout in user creation endpoint
refactor(db): extract query builder from repositoryBefore submitting the review request:
- [ ] All pre-flight checks passing (Phase 1)
- [ ] Self-review completed, no issues found (Phase 2)
- [ ] Change summary includes what, why, and impact (Phase 3)
- [ ] Test evidence shows actual output, not "tests pass" (Phase 4)
- [ ] At least one review focus area identified (Phase 5)
- [ ] PR description follows template (Phase 6)
- [ ] PR title follows convention
- [ ] No TODO/FIXME in diff
- [ ] No debug artifacts in diff
- [ ] Branch is up-to-date with baseCannot check all boxes? Do not submit the review request.
| Thought | Reality |
|---|---|
| "Tests are probably passing" | Run them. "Probably" is not evidence. |
| "The diff is small, no need to self-review" | Small diffs cause big bugs. 2 minutes to check. |
| "I'll add test evidence later" | Reviewers need it NOW to evaluate correctness. |
| "The reviewer will figure out what changed" | That's YOUR job. Respect their time. |
| "No breaking changes... I think" | Check consumers. "I think" is not analysis. |
| "I'll just describe what I did" | Describe WHY, not just WHAT. |
| "This is urgent, skip preparation" | Urgency increases risk. Preparation prevents rework. |
| "The PR is too big to summarize" | Split it. If you can't summarize it, reviewers can't review it. |
| Skill | Relationship |
|---|---|
pr-review | Complement — pr-review is for CONDUCTING reviews; this skill is for REQUESTING them |
deployment-checklist | Sequential — deploy after review approval |
test-driven-development | Supporting — TDD produces the test evidence this skill requires |
security-audit | Supporting — security audit informs review focus areas |
~30 seconds. Free. No account. Every finding cites a rule and a line of evidence.