8.9 KiB
8.9 KiB
name, description
| name | description |
|---|---|
| stackshift.review | Perform comprehensive code review across multiple dimensions - correctness, standards, security, performance, and testing. Returns APPROVED/NEEDS CHANGES/BLOCKED with specific feedback. |
Code Review
Comprehensive multi-dimensional code review with actionable feedback.
Usage
# Review recent changes
/stackshift.review
# Review specific feature
/stackshift.review "pricing display feature"
# Review before deployment
/stackshift.review "OAuth2 implementation before production"
What This Reviews
🔍 Correctness
- Does it work as intended?
- Are all requirements met?
- Logic errors or edge cases?
- Matches specification requirements?
📏 Standards Compliance
- Follows project conventions?
- Coding style consistent?
- Documentation adequate?
- Aligns with constitution principles?
🔒 Security Assessment
- No obvious vulnerabilities?
- Proper input validation?
- Secure data handling?
- Authentication/authorization correct?
⚡ Performance Review
- Efficient implementation?
- Resource usage reasonable?
- Scalability considerations?
- Database queries optimized?
🧪 Testing Validation
- Adequate test coverage?
- Edge cases handled?
- Error conditions tested?
- Integration tests included?
Review Process
Step 1: Identify What Changed
echo "🔍 Identifying changes to review..."
# Get recent changes
git diff HEAD~1 --name-only > changed-files.txt
# Show files to review
echo "Files to review:"
cat changed-files.txt | sed 's/^/ - /'
# Count by type
BACKEND_FILES=$(grep -E "api/|src/.*\.ts$|lib/" changed-files.txt | wc -l)
FRONTEND_FILES=$(grep -E "site/|pages/|components/.*\.tsx$" changed-files.txt | wc -l)
TEST_FILES=$(grep -E "\.test\.|\.spec\.|__tests__" changed-files.txt | wc -l)
echo ""
echo "📊 Changes breakdown:"
echo " Backend: $BACKEND_FILES files"
echo " Frontend: $FRONTEND_FILES files"
echo " Tests: $TEST_FILES files"
echo ""
Step 2: Review Each Dimension
echo "🔍 Performing multi-dimensional review..."
echo ""
# Correctness Check
echo "✓ Correctness Review"
# Check if tests exist for changed files
for file in $(cat changed-files.txt); do
if [[ ! "$file" =~ \.test\. && ! "$file" =~ \.spec\. ]]; then
# Look for corresponding test file
TEST_FILE="${file%.ts}.test.ts"
if [ ! -f "$TEST_FILE" ]; then
echo " ⚠️ Missing tests for: $file"
echo "ISSUE: No test coverage for $file" >> review-issues.log
fi
fi
done
# Check against spec requirements
echo " Checking against specifications..."
# Implementation reviews code against spec requirements
echo ""
# Standards Compliance
echo "✓ Standards Review"
# Check for common issues
grep -rn "console.log\|debugger" $(cat changed-files.txt) > debug-statements.log 2>/dev/null
if [ -s debug-statements.log ]; then
echo " ⚠️ Debug statements found:"
cat debug-statements.log
echo "ISSUE: Debug statements in production code" >> review-issues.log
fi
# Check for TODO/FIXME
grep -rn "TODO\|FIXME\|XXX\|HACK" $(cat changed-files.txt) > todos.log 2>/dev/null
if [ -s todos.log ]; then
echo " ⚠️ Unresolved TODOs found:"
cat todos.log
echo "ISSUE: Unresolved TODO comments" >> review-issues.log
fi
echo ""
# Security Assessment
echo "✓ Security Review"
# Check for common security issues
grep -rn "eval(\|innerHTML\|dangerouslySetInnerHTML" $(cat changed-files.txt) > security-issues.log 2>/dev/null
if [ -s security-issues.log ]; then
echo " ❌ Security concerns found:"
cat security-issues.log
echo "CRITICAL: Security vulnerability" >> review-issues.log
fi
# Check for hardcoded secrets/credentials
grep -rn "password.*=\|api.*key.*=\|secret.*=" $(cat changed-files.txt) > credentials.log 2>/dev/null
if [ -s credentials.log ]; then
echo " ❌ Potential hardcoded credentials:"
cat credentials.log
echo "CRITICAL: Hardcoded credentials" >> review-issues.log
fi
echo ""
# Performance Review
echo "✓ Performance Review"
# Check for common performance issues
grep -rn "for.*in.*forEach\|while.*push\|map.*filter.*map" $(cat changed-files.txt) > performance-issues.log 2>/dev/null
if [ -s performance-issues.log ]; then
echo " ⚠️ Potential performance issues:"
cat performance-issues.log | head -5
echo "ISSUE: Inefficient loops or chaining" >> review-issues.log
fi
echo ""
# Testing Validation
echo "✓ Testing Review"
# Check test quality
if [[ "$TEST_FILES" -gt 0 ]]; then
# Check for proper test structure
grep -rn "describe\|it(\|test(" $(grep "\.test\.\|\.spec\." changed-files.txt) > test-structure.log 2>/dev/null
TEST_COUNT=$(grep -c "it(\|test(" test-structure.log || echo "0")
echo " Test cases found: $TEST_COUNT"
if [[ "$TEST_COUNT" -lt 5 ]]; then
echo " ⚠️ Low test coverage detected"
echo "ISSUE: Insufficient test cases (< 5)" >> review-issues.log
fi
else
echo " ⚠️ No tests added for changes"
echo "ISSUE: No test files modified" >> review-issues.log
fi
echo ""
Step 3: Generate Review Report
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
echo "📋 Review Report"
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
echo ""
TOTAL_ISSUES=$(wc -l < review-issues.log 2>/dev/null || echo "0")
CRITICAL_ISSUES=$(grep -c "CRITICAL" review-issues.log 2>/dev/null || echo "0")
if [[ "$TOTAL_ISSUES" == "0" ]]; then
echo "### ✅ APPROVED"
echo ""
echo "**Strengths:**"
echo "- All quality checks passed"
echo "- Spec compliance validated"
echo "- Security review clean"
echo "- Performance acceptable"
echo "- Test coverage adequate"
echo ""
echo "**Decision:** Ready for next phase/deployment"
else
if [[ "$CRITICAL_ISSUES" -gt 0 ]]; then
echo "### 🚫 BLOCKED"
echo ""
echo "**Critical Issues Found:** $CRITICAL_ISSUES"
echo ""
echo "Critical issues must be resolved before proceeding:"
else
echo "### 🔄 NEEDS CHANGES"
echo ""
echo "**Issues Found:** $TOTAL_ISSUES"
echo ""
echo "Issues requiring attention:"
fi
echo ""
cat review-issues.log | sed 's/^/ - /'
echo ""
echo "**Recommendations:**"
echo " 1. Address critical security/spec violations first"
echo " 2. Run /stackshift.validate --fix to auto-resolve technical issues"
echo " 3. Add missing test coverage"
echo " 4. Remove debug statements and TODOs"
echo " 5. Re-run review after fixes"
echo ""
if [[ "$CRITICAL_ISSUES" -gt 0 ]]; then
echo "**Decision:** BLOCKED - Cannot proceed until critical issues resolved"
else
echo "**Decision:** NEEDS CHANGES - Address issues before finalizing"
fi
fi
echo ""
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
# Cleanup
rm -f changed-files.txt debug-statements.log todos.log security-issues.log \
credentials.log performance-issues.log test-structure.log review-issues.log
Review Categories
✅ APPROVED
- All critical issues resolved
- Meets quality standards
- Ready for next phase/deployment
🔄 NEEDS CHANGES
- Issues found requiring fixes
- Specific feedback provided
- Return to implementation
🚫 BLOCKED
- Fundamental issues requiring redesign
- Critical security vulnerabilities
- Major spec violations
Quality Standards
- No rubber stamping - Actually review, don't just approve
- Specific feedback - Actionable recommendations with line numbers
- Priority classification - Critical, Important, Suggestion
- Evidence-based - Cite specific files, lines, patterns
Integration with StackShift
Auto-runs after:
- Gear 6 completion
/stackshift.validatefinds issues- Before final commit
Manual usage:
# Before committing
/stackshift.review
# After implementing feature
/stackshift.review "vehicle details feature"
# Before deployment
/stackshift.review "all changes since last release"
Example Output
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
📋 Review Report
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
### 🔄 NEEDS CHANGES
**Issues Found:** 3
Issues requiring attention:
- ISSUE: Debug statements in production code
- ISSUE: No test coverage for api/handlers/pricing.ts
- ISSUE: Inefficient loops or chaining in data-processor.ts
**Recommendations:**
1. Remove console.log from api/handlers/pricing.ts:45
2. Add test file: api/handlers/pricing.test.ts
3. Refactor nested loops in data-processor.ts:78-82
4. Re-run review after fixes
**Decision:** NEEDS CHANGES - Address issues before finalizing
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Quality assurance before every finalization!