--- name: stackshift.review description: 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 ```bash # 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 ```bash 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 ```bash 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 ```bash 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.validate` finds issues - Before final commit **Manual usage:** ```bash # 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!**