一键导入
code-review-checklist
// Structured code review criteria for pre-implementation plan review (Critic) and post-implementation security/quality review. Covers security, performance, maintainability, and correctness with severity ratings.
// Structured code review criteria for pre-implementation plan review (Critic) and post-implementation security/quality review. Covers security, performance, maintainability, and correctness with severity ratings.
| name | code-review-checklist |
| description | Structured code review criteria for pre-implementation plan review (Critic) and post-implementation security/quality review. Covers security, performance, maintainability, and correctness with severity ratings. |
| license | MIT |
| metadata | {"author":"groupzer0","version":"1.0"} |
Systematic review criteria for evaluating code and plans. Use this skill when:
This skill supports two review phases:
| Phase | Agent | Focus | Documents |
|---|---|---|---|
| Pre-Implementation | Critic | Plan quality, clarity, completeness | planning/*.md |
| Post-Implementation | Security, Architect | Code quality, security, architecture | Source code |
| Check | Question | Finding Severity |
|---|---|---|
| Presence | Does plan have clear value statement in user story format? | CRITICAL if missing |
| Clarity | Is "So that" outcome measurable or verifiable? | HIGH if vague |
| Alignment | Does value support Master Product Objective? | CRITICAL if drift |
| Directness | Is value delivered directly, not deferred? | HIGH if deferred |
| Check | Question | Finding Severity |
|---|---|---|
| Scope | Are boundaries clearly defined? | MEDIUM |
| Deliverables | Are all deliverables listed with acceptance criteria? | HIGH |
| Dependencies | Are dependencies identified and sequenced? | MEDIUM |
| Risks | Are risks documented with mitigations? | LOW |
| Version | Is semver bump specified with rationale? | MEDIUM |
| Check | Question | Finding Severity |
|---|---|---|
| No Code | Does plan avoid prescriptive code? | LOW |
| No How | Does plan focus on WHAT/WHY, not HOW? | LOW |
| Architecture | Does plan respect architectural constraints? | HIGH |
| Category | Check | Severity |
|---|---|---|
| Input Validation | All user input validated server-side? | CRITICAL |
| Authentication | Auth checks on all protected routes? | CRITICAL |
| Authorization | RBAC/ownership verified before access? | CRITICAL |
| Secrets | No hardcoded credentials or keys? | CRITICAL |
| SQL/Injection | Parameterized queries used? | CRITICAL |
| XSS | Output encoding applied? | HIGH |
| CSRF | Tokens on state-changing requests? | HIGH |
| Logging | Security events logged without sensitive data? | MEDIUM |
| Dependencies | No known CVEs in dependencies? | Varies |
| Category | Check | Severity |
|---|---|---|
| N+1 Queries | Batch fetches instead of loops? | HIGH |
| Pagination | Large datasets paginated? | HIGH |
| Caching | Appropriate caching strategy? | MEDIUM |
| Async | Long operations non-blocking? | MEDIUM |
| Resource Limits | Bounded allocations? | HIGH |
| Category | Check | Severity |
|---|---|---|
| Naming | Clear, descriptive names? | LOW |
| Complexity | Cyclomatic complexity < 10? | MEDIUM |
| Coupling | Low coupling between modules? | MEDIUM |
| Documentation | Public APIs documented? | LOW |
| Error Handling | Errors handled, not swallowed? | HIGH |
| Tests | Adequate coverage for changes? | HIGH |
| Category | Check | Severity |
|---|---|---|
| Boundaries | Module boundaries respected? | HIGH |
| Patterns | Established patterns followed? | MEDIUM |
| Dependencies | Dependency direction correct? | HIGH |
| Single Responsibility | Classes/modules focused? | MEDIUM |
| Severity | Response | Examples |
|---|---|---|
| CRITICAL | Block until fixed | Auth bypass, SQL injection, no value statement |
| HIGH | Fix before merge | Missing validation, N+1 queries, unclear scope |
| MEDIUM | Fix in current cycle | Code smells, missing docs, minor coupling |
| LOW | Track for later | Style issues, optimization opportunities |
Document findings consistently:
### [ID]: [Brief Title]
- **Severity**: CRITICAL / HIGH / MEDIUM / LOW
- **Status**: OPEN / ADDRESSED / RESOLVED / DEFERRED
- **Location**: [file:line or plan section]
- **Description**: [What is the issue?]
- **Impact**: [Why does this matter?]
- **Recommendation**: [How to fix?]
agent-output/critiques/security-patterns skill for detectionagent-output/security/architecture-patterns skillsystem-architecture.md when issues foundCode review checklist, severity definitions, and document templates. Load when performing code reviews or defining review criteria.
Unified document lifecycle management. Defines terminal statuses, unified numbering via .next-id, close procedures, and orphan detection. Load at session start.
Unified Memory Contract for Flowbaby integration. Defines when and how to retrieve and store memory. Load at session start - memory is core to agent reasoning, not optional.
TDD workflow and test strategy patterns including test pyramid, coverage strategies, mocking approaches, and anti-patterns. Load when writing tests, designing test strategies, or reviewing test coverage.
Common software architecture patterns, ADR templates, and anti-pattern detection. Supports architectural review, design decisions, and system documentation.
Version management, release verification, and deployment procedures for software releases. Includes semver guidance, version consistency checks, and platform-specific constraints.