name: Code Review Standards description: This skill should be used when the user asks to "review code", "perform code review", "check code quality", "review PR", "provide code feedback", or needs guidance on code review best practices and standards in k2-dev workflows. version: 0.2.0

Code Review Standards Skill

Overview

Code review is a critical quality control process that catches bugs, ensures standards compliance, and facilitates knowledge sharing.

Review Objectives: Ensure code quality and maintainability, catch bugs and logic errors, validate security practices, enforce project standards, share knowledge.

Review Process

Four-Pass Review Method

Pass 1: High-Level Understanding

Read PR description and ticket context
Understand what the code is supposed to do
Review architectural approach
Identify scope and boundaries

Pass 2: Line-by-Line Detailed Review

Read every changed line
Check logic correctness
Identify potential bugs
Note style issues

Pass 3: Standards Validation

Validate against AGENTS.md quality gates
Verify constitution.md principles
Ensure project standards followed

Pass 4: Architectural Assessment

Evaluate design decisions
Check for code smells
Assess maintainability
Consider alternatives

Review Checklist

Code Quality

Readability:

Code is self-documenting
Variable names are descriptive
Functions appropriately sized (<50 lines ideal)
Complex logic has explanatory comments
No commented-out code

Structure:

Proper separation of concerns
DRY principle followed (no duplication)
Single Responsibility Principle
Appropriate abstraction levels
Consistent code style

Error Handling:

Logic and Correctness

Functionality:

Data Handling:

Security

OWASP Top 10 Security Checklist (Detailed)

For every PR, validate:

Injection:
- SQL queries use parameterized statements or ORMs
- NoSQL queries don't use string concatenation
- OS commands don't use unsanitized user input
- LDAP queries are parameterized
Broken Authentication:
- Passwords are hashed (bcrypt, Argon2)
- Session tokens are secure, random, and expire
- Multi-factor authentication is implemented (if required)
- No credentials in code or config
Sensitive Data Exposure:
- No API keys, passwords, or secrets in code
- Sensitive data encrypted in transit (HTTPS/TLS)
- Sensitive data encrypted at rest
- No sensitive data in logs or error messages
XML External Entities (XXE):
- XML parsing disables external entity processing
- XML libraries are configured securely
Broken Access Control:
- Authorization checks before sensitive operations
- Users can't access others' data without permission
- Admin functions require admin privileges
- CORS policies are restrictive
Security Misconfiguration:
- No default passwords or credentials
- Error messages don't leak sensitive info
- Security headers are set (CSP, X-Frame-Options, etc.)
- Unnecessary features/services are disabled
Cross-Site Scripting (XSS):
- User input is escaped before rendering
- HTML sanitization is applied to rich content
- Content Security Policy is used
- No dangerouslySetInnerHTML or equivalent without sanitization
Insecure Deserialization:
- Deserialization is from trusted sources only
- Input validation before deserialization
- Type checks on deserialized objects
Using Components with Known Vulnerabilities:
- Dependencies are up-to-date
- No known CVEs in dependencies
- Dependency versions are locked
Insufficient Logging & Monitoring:
- Security events are logged
- Errors are logged (without sensitive data)
- Audit trail for sensitive operations

Input Validation:

Authentication & Authorization:

Data Protection:

Performance

Efficiency:

Resource Usage:

Testing

Coverage:

Meets minimum coverage requirement (typically 80%)
Critical paths fully tested
Edge cases covered
Error conditions tested
Integration tests present

Test Quality:

Documentation

Code Documentation:

External Documentation:

Project Standards

AGENTS.md Compliance:

constitution.md Compliance:

Core principles upheld
Constraints respected
Security policies followed
Performance requirements met

Providing Feedback

Feedback Structure

Standard format:

**[Severity]** [Category]: [Issue]

[Explanation]

[Suggestion]

[Example or reference]

Severity levels:

P0 (Critical): Security vulnerabilities, data loss, breaking changes
P1 (Important): Logic errors, quality gate violations, architecture issues
P2 (Minor): Style issues, optimization opportunities, refactoring suggestions
Suggestion: Nice-to-haves, alternative approaches, learning opportunities

Reference: See k2-dev-reference.md#review-severity-levels

Example Feedback

P0 - Security Issue:

**P0** Security: SQL Injection Vulnerability

The search query is directly concatenated into the SQL statement, allowing injection attacks.

\```typescript
// Current (vulnerable)
const query = `SELECT * FROM users WHERE name = '${searchTerm}'`;

// Fix: Use parameterized queries
const query = 'SELECT * FROM users WHERE name = ?';
const results = await db.query(query, [searchTerm]);
\```

Reference: OWASP SQL Injection Prevention Cheat Sheet

P1 - Logic Error:

**P1** Logic: Off-by-One Error in Pagination

The pagination logic will skip the last item on each page due to incorrect boundary condition.

\```typescript
// Current (incorrect)
const end = start + pageSize;  // Should be exclusive

// Fix
const end = Math.min(start + pageSize, totalItems);
\```

Add test case TC-045 to catch this.

Tone and Approach

DO: ✅ Be specific and objective ✅ Explain the reasoning ✅ Provide examples and references ✅ Suggest solutions, not just problems ✅ Distinguish between must-fix and nice-to-have ✅ Acknowledge good practices ✅ Ask questions if unclear

DON'T: ❌ Be vague ("this looks wrong") ❌ Be condescending or dismissive ❌ Focus only on negatives ❌ Nitpick style in isolation ❌ Demand specific implementations ❌ Leave feedback without explanation

Review Iterations

Iteration 1

Reviewer:

Conducts four-pass review
Categorizes findings by severity
Provides comprehensive feedback on GitHub PR
Requests changes if issues found
Approves if all checks pass

Engineer:

Reads all feedback carefully
Addresses all P0 and P1 issues
Considers P2 and suggestions
Pushes fixes
Responds to comments
Requests re-review

Iteration 2

Reviewer:

Reviews changes since last review
Verifies P0/P1 issues resolved
Checks for new issues introduced
Approves if satisfied
If still issues, prepare for follow-up tickets

Engineer:

Addresses remaining feedback
If can't resolve in reasonable time:
- Create follow-up tickets (P0 for critical)
- Document in PR
- Request approval to merge with tickets

After 2 Iterations

If issues remain after 2 iterations:

Create follow-up tickets:

# Critical issues
bd create "Fix authentication bypass vulnerability" \
  -d "Critical issue from PR #456 review. Details: [...]" \
  -p P0

# Important issues
bd create "Refactor validation logic" \
  -d "Code quality issue from PR #456. Details: [...]" \
  -p P1

Document in PR:

## Outstanding Issues

After 2 review iterations, the following issues remain:

### Created Follow-Up Tickets
- **P0:** beads-789 - Fix authentication bypass (blocking next release)
- **P1:** beads-790 - Refactor validation logic

### Decision
Merging this PR to maintain forward progress. Critical issues tracked in P0 ticket and will be addressed immediately.

Reference: See k2-dev-reference.md#beads-cli-commands for beads task creation.

K2-Dev Review Workflow

Reviewer Agent Process

Read context:

bd show beads-123
bd comments beads-123
cat AGENTS.md constitution.md

Review code:

gh pr view 456 --web
gh pr diff 456

Provide feedback on GitHub:

Inline comments on specific lines
Summary comment with categorized issues
Use severity labels (P0/P1/P2/Suggestion)

Report to Technical Lead:

Review complete for beads-123 (PR #456):
- P0 issues: 1 (security)
- P1 issues: 2 (logic errors)
- P2 issues: 3 (style)
- Requesting changes

Re-review after changes:

Check if issues addressed
Verify no new issues introduced
Approve or continue iteration

Reference: See k2-dev-reference.md#github-cli-gh-commands for gh pr commands.

Best Practices

DO

✅ Review within 24 hours of PR creation ✅ Block time for focused review ✅ Review all changed files (including tests and docs) ✅ Consider broader impact ✅ Be constructive and helpful

DON'T

❌ Rush through review or review when tired ❌ Only review main files (skip tests/docs) ❌ Be dismissive or condescending ❌ Make personal attacks ❌ Ignore engineer's responses

Review Templates

Initial Review Comment

## Review Started

Beginning review of PR #456 for beads-123.

Will be checking:
✓ Code quality and logic
✓ Security vulnerabilities
✓ Standards compliance (AGENTS.md, constitution.md)
✓ Test coverage
✓ Documentation

Feedback coming soon.

Approval Comment

## ✅ Approved

Great work on this implementation! All quality gates pass.

### Highlights
- Excellent test coverage (95%)
- Clean, maintainable code
- Good security practices
- Clear documentation

### Standards Validation
✅ AGENTS.md quality gates pass
✅ constitution.md principles upheld

Ready to merge!

Request Changes Comment

## 🔄 Changes Requested

Found some issues that need addressing before merge. Overall approach is solid!

### Must Fix (P0/P1)
1. [P0] SQL injection vulnerability (line 123)
2. [P1] Logic error in pagination (line 234)

### Should Consider (P2)
3. Inconsistent error handling (throughout)

### Suggestions
- Consider extracting validation logic to utility

Please address P0/P1 issues and push updates. Let me know if you have questions!

Follow these code review standards to maintain high code quality and catch issues early in k2-dev development workflows.

Reference: See k2-dev-reference.md for priority levels, common patterns, and gh commands.

Name	code-review-standards
Description	This skill should be used when the user asks to "review code", "perform code review", "check code quality", "review PR", "provide code feedback", or needs guidance on code review best practices and standards in k2-dev workflows.

code-review-standards

SKILL.md