Code Review Expert
Scannednpx machina-cli add skill hoangnguyen0403/agent-skills-standard/code-review --openclawCode Review Expert
Priority: P1 (OPERATIONAL)
Act as a Principal Engineer. Focus on logic, security, and architecture. Be constructive.
Review Principles
- Substance > Style: Ignore formatting (leave to linters). Find bugs & design flaws.
- Questions > Commands: "Does this handle null?" vs "Fix this."
- Readability: Group by
[BLOCKER],[MAJOR],[NIT]. - Cross-Check: Enforce P0 rules from active framework skills (e.g.
flutter/security,react/hooks).
Review Checklist (Summary)
- Shields Up (Security): Injection? Auth? Secrets?
- Performance: Big O? N+1 queries? Memory leaks?
- Correctness: Requirements met? Edge cases?
- Clean Code: DRY? SOLID? Intent-revealing names?
See references/checklist.md for full inspection list.
Output Format (Mandatory)
1. Summary: One sentence on overall quality/impact. 2. Categorized Findings:
### 🔴 [BLOCKER]
- **File**: `auth.ts`
- **Issue**: SQL Injection risk in `login`.
- **Suggestion**: Use parameterized query.
```typescript
// Recommended Fix
db.query('SELECT * FROM users WHERE id = $1', [userId]);
```
🟢 [NIT]
- File:
utils.ts - Issue: Rename
dtodaysfor clarity.
See references/output-format.md for templates.
Anti-Patterns
- No Nitpicking: Don't flood with minor style comments.
- No Vague Demands: "Fix this" -> Explain why and how.
- No Ghosting: Always review tests and edge cases.
Source
git clone https://github.com/hoangnguyen0403/agent-skills-standard/blob/develop/.github/skills/common/code-review/SKILL.mdView on GitHub Overview
Act as a Principal Engineer to assess logic, security, and architecture in code reviews. Emphasize substance over style, surface-level fixes, and ensure findings are actionable. Group findings into BLOCKER, MAJOR, and NIT, and cross-check against active framework rules like flutter/security or react/hooks.
How This Skill Works
Follow the Review Principles and Checklist to guide every assessment. Start with security, performance, correctness, and clean code, then document issues using a structured output format with file references and concrete suggestions. Prioritize blockers, then major issues, and finish with nitpicks to improve readability and maintainability.
When to Use It
- When code introduces potential security risks such as injections, broken authentication, or leaked secrets.
- When performance concerns are present, e.g., high complexity, N+1 queries, or memory leaks.
- When correctness and edge cases must be verified against requirements and expected behavior.
- When code readability and maintainability are at risk, necessitating clearer intent and naming.
- When aligning with architectural standards and cross-framework rules (e.g., flutter/security, react/hooks).
Quick Start
- Step 1: Open the PR and review the changes with a focus on security, correctness, and architecture.
- Step 2: Classify findings into BLOCKER, MAJOR, and NIT; capture file references and concrete fixes with rationale.
- Step 3: Present feedback using the structured output format with actionable code changes and tests.
Best Practices
- Prioritize security, correctness, and architecture over cosmetic formatting.
- Ask clarifying questions instead of giving commands; explain why a change is needed.
- Structure feedback by BLOCKER, MAJOR, and NIT with concrete file references and fixes.
- Cross-check against active framework rules (e.g., flutter/security, react/hooks) and tests.
- Always review tests and edge cases, and provide actionable, testable fixes with rationale.
Example Use Cases
- Blocker in auth.ts: SQL Injection risk in login; fix with parameterized query (e.g., db.query('SELECT * FROM users WHERE id = $1', [userId])) and proper input validation.
- Nit in utils.ts: Rename d to days for clarity.
- Major: N+1 query detected in user profile fetch; refactor to use join or eager loading to reduce database calls.
- Edge: Missing null checks in login flow; add null-safe guards and input validation.
- Clean Code: Rename ambiguous function names for clarity and add unit tests for critical paths.
