astro44/review-code-review

review-code-review - Automated Code Review

Quick code review checking for common issues.

Input Schema

{
  "project_dir": "/path/to/project",
  "ticket_id": "TICKET-XXX",
  "files": ["src/components/Foo.js"],
  "checks": ["security", "design_tokens", "accessibility", "data_contracts"]
}

Review Checks

Security (CRITICAL)

| Pattern | Severity | Fix | |---------|----------|-----| | innerHTML = with variable | HIGH | Use textContent or DOM APIs | | eval() with external data | CRITICAL | Remove, use safe alternatives | | document.write() | MEDIUM | Use DOM APIs | | Hardcoded credentials | CRITICAL | Move to env vars | | onclick="..." inline handlers | MEDIUM | Use addEventListener |

Design Tokens

| Pattern | Severity | Fix | |---------|----------|-----| | Hardcoded hex colors (#fff) | HIGH | Use var(--color-*) | | Hardcoded px values | MEDIUM | Use var(--spacing-*) | | Magic numbers in CSS | MEDIUM | Extract to design tokens | | Inline styles with colors | HIGH | Use CSS classes |

Accessibility

| Pattern | Severity | Fix | |---------|----------|-----| | <img> without alt | HIGH | Add alt attribute | | Icon without aria-label | MEDIUM | Add label for screen readers | | Non-semantic elements for buttons | HIGH | Use <button> | | Missing role on interactive elements | MEDIUM | Add appropriate role |

P4.2: Data Contracts (NEW)

Validates that data-* attributes referenced in JS/CSS exist in HTML.

| Pattern | Severity | Fix | |---------|----------|-----| | data-* in JS not in HTML | HIGH | Add attribute to HTML or remove JS reference | | CSS selector [data-*] not in HTML | HIGH | Add element or remove selector | | querySelector('[data-xyz]') with no match | HIGH | Verify selector exists in DOM | | document.querySelectorAll('[data-*]') returning empty | MEDIUM | Check HTML has elements | | JS expects .dataset.xyz but element lacks data-xyz | HIGH | Add data attribute to HTML |

Cross-Reference Validation:

When reviewing UI components, cross-reference:

1. All data-* attributes used in JS exist in HTML
2. All CSS selectors targeting [data-*] have matching HTML elements
3. Cross-reference against CATALOG_PENDING.json identifiers

Detection Patterns:

// JS patterns that reference data-* attributes
const DATA_JS_PATTERNS = [
  /querySelector\(['"]?\[data-([^\]]+)\]/g,       // querySelector('[data-xyz]')
  /querySelectorAll\(['"]?\[data-([^\]]+)\]/g,   // querySelectorAll('[data-xyz]')
  /\.dataset\.(\w+)/g,                            // element.dataset.xyz
  /getAttribute\(['"]data-([^'"]+)['"]\)/g,       // getAttribute('data-xyz')
];

// CSS patterns that reference data-* attributes
const DATA_CSS_PATTERNS = [
  /\[data-([^\]]+)\]/g,                           // [data-xyz]
  /\[data-([^\]]+)=['"][^'"]*['"]\]/g,           // [data-xyz="value"]
];

Validation Output:

{
  "category": "data_contracts",
  "severity": "HIGH",
  "message": "JS references [data-metric-card] but no matching element in HTML",
  "evidence": {
    "js_file": "src/js/metrics.js",
    "js_line": 42,
    "selector": "[data-metric-card]",
    "html_files_checked": ["src/index.html", "src/pages/dashboard.html"],
    "matches_found": 0
  },
  "fix": "Add `data-metric-card` attribute to element in HTML, or remove JS reference"
}

When HTML Not Available:

If JS references data-* but HTML files are not provided, flag as:

{
  "category": "data_contracts",
  "severity": "HIGH",
  "message": "Cannot verify data-* contract - HTML not provided",
  "evidence": {
    "js_file": "src/js/component.js",
    "data_attributes_used": ["data-chart", "data-metric-card", "data-toggle"],
    "html_files_provided": []
  },
  "fix": "Provide HTML files to verify data-* contracts"
}

Output Format

{
  "skill": "review-code-review",
  "status": "approved|rejected|needs_fixes",
  "files_reviewed": 3,
  "issues": [
    {
      "file": "src/components/Foo.js",
      "line": 42,
      "severity": "HIGH",
      "category": "security",
      "message": "innerHTML with variable data - XSS risk",
      "fix": "Use textContent for plain text or sanitize input"
    }
  ],
  "summary": {
    "critical": 0,
    "high": 1,
    "medium": 2,
    "low": 0
  },
  "errors": [],
  "warnings": [],
  "next_action": "proceed|fix|review"
}

Decision Logic

| Issues Found | Status | Next Action | |--------------|--------|-------------| | 0 issues | approved | proceed | | Only LOW/MEDIUM | approved | proceed (with warnings) | | Any HIGH | needs_fixes | fix | | Any CRITICAL | rejected | fix (mandatory) |

Error Handling

| Error | Cause | Resolution | |-------|-------|------------| | file_not_found | File path doesn't exist | Check ticket files_created | | parse_error | Invalid JS/CSS syntax | Run linter first | | binary_file | Attempted to review image/binary | Skip, return warning | | permission_denied | Can't read file | Check file permissions | | empty_file | File has no content | Skip with warning |

Usage Examples

Security-focused review:

{
  "project_dir": "/projects/oxygen_site",
  "ticket_id": "TICKET-OXY-001",
  "files": [
    "src/components/UserInput.js",
    "src/utils/api.js"
  ],
  "checks": ["security"]
}

Full review (all checks):

{
  "project_dir": "/projects/oxygen_site",
  "ticket_id": "TICKET-OXY-002",
  "files": [
    "src/components/Dashboard.jsx",
    "src/styles/dashboard.css",
    "src/pages/dashboard.html"
  ],
  "checks": ["security", "design_tokens", "accessibility", "data_contracts"]
}

Design system compliance:

{
  "project_dir": "/projects/design-system",
  "ticket_id": "TICKET-DS-001",
  "files": [
    "src/components/Button.jsx",
    "src/components/Card.jsx"
  ],
  "checks": ["design_tokens", "accessibility"]
}

Single file quick check:

{
  "project_dir": "/projects/api-client",
  "ticket_id": "TICKET-API-001",
  "files": ["src/auth/login.js"],
  "checks": ["security"]
}

Token Efficiency

• Pattern-based scanning, not full AST
• Returns only actionable issues
• ~5-15 second execution
• Batches file reads for speed