Loading...
Loading...
Pre-commit code quality review workflow. Use before committing changes to verify code quality, security, testing coverage, and adherence to project conventions. Supports both automated checking and interactive review modes.
npx skill4agent add ar4mirez/samuel code-review| Trigger | Mode | Description |
|---|---|---|
| Pre-Commit | Automated | Before any commit |
| PR Review | Interactive | During pull request review |
| Feature Complete | Both | After FEATURE/COMPLEX mode completion |
| Code Handoff | Interactive | Before transferring ownership |
| Quality Gate | Automated | CI/CD pipeline integration |
Phase 1: Code Quality Guardrails
↓
Phase 2: Security Guardrails
↓
Phase 3: Testing Guardrails
↓
Phase 4: Git Hygiene
↓
Phase 5: Report GenerationCheck: Count lines in each function/method
Pass: All functions ≤ 50 lines
Warn: Functions 40-50 lines (approaching limit)
Fail: Any function > 50 linesCheck: Count lines in each file
Pass: Files within limits
Warn: Files at 80%+ of limit
Fail: Files exceeding limits| Type | Limit | 80% Warning |
|---|---|---|
| Components | 200 | 160 |
| Tests | 300 | 240 |
| Utilities | 150 | 120 |
| Other | 300 | 240 |
Check: Analyze control flow (if, for, while, switch, &&, ||)
Pass: All functions ≤ 10
Warn: Functions 8-10
Fail: Any function > 10Check: Verify exported functions have types
Pass: All exports typed
Warn: Internal functions missing types
Fail: Exported functions missing typesCheck: Verify docstrings/JSDoc on exports
Pass: All exports documented
Warn: Documentation exists but incomplete
Fail: Exported functions undocumentedCheck: Scan for violations
Pass: None found
Warn: Minor violations (1-2 magic numbers)
Fail: Multiple violationsCheck: Identify input sources (API params, form data, URL params)
Pass: All inputs validated with schema or type checks
Warn: Validation exists but not comprehensive
Fail: Raw user input used directlyCheck: Scan for SQL/query construction
Pass: All queries parameterized
Fail: String concatenation in queries// FAIL: String concatenation
`SELECT * FROM users WHERE id = ${id}`
// PASS: Parameterized
db.query('SELECT * FROM users WHERE id = $1', [id])Check: Scan for secret patterns
Pass: No secrets detected
Fail: Hardcoded secrets foundsk_live_pk_live_api_key=password=passwd=token=bearer-----BEGIN RSA PRIVATE KEY-----Check: Identify file operations
Pass: Path validation/sanitization present
Fail: Direct user input in file paths// FAIL: Directory traversal possible
fs.readFile(userInput)
// PASS: Validated
const safePath = path.join(baseDir, path.basename(userInput))Check: Identify async calls (fetch, DB queries, external APIs)
Pass: Timeouts configured
Warn: Some operations without timeout
Fail: No timeout handlingCheck: Review coverage report
Pass: Meets thresholds
Warn: 70-80% business / 50-60% overall
Fail: Below thresholdsCheck: Map public functions to test files
Pass: All public APIs tested
Warn: Most tested (>80%)
Fail: Significant gaps (<80%)Check: If fix, verify test added
Pass: Regression test present
Fail: No regression test for bug fixCheck: Review test cases for boundaries
Pass: Null, empty, boundary values tested
Warn: Some edge cases missing
Fail: No edge case testingCheck: Tests can run in any order
Pass: Tests isolated
Fail: Tests depend on execution ordertype(scope): descriptionCheck: Verify commit message format
Pass: Follows convention
Fail: Non-conventional formatCheck: Review commit scope
Pass: Single logical change
Warn: Related changes (acceptable)
Fail: Unrelated changes bundledCheck: Scan staged files
Pass: No sensitive data
Fail: Secrets, credentials, or PII found## Code Review Report
**Date**: 2025-01-15
**Files Reviewed**: 12
**Status**: ⚠️ WARNINGS (3 issues)
### Summary
| Category | Status | Issues |
|----------|--------|--------|
| Code Quality | ✅ Pass | 0 |
| Security | ⚠️ Warn | 2 |
| Testing | ✅ Pass | 0 |
| Git Hygiene | ⚠️ Warn | 1 |
### Issues Found
#### Security Warnings
1. **Missing timeout** in `src/api/fetch.ts:42`
- External API call without timeout
- Recommendation: Add 30s timeout
2. **Input validation** in `src/routes/users.ts:15`
- Query parameter used without validation
- Recommendation: Add Zod schema
#### Git Hygiene Warnings
1. **Large commit scope**
- 8 files changed across 3 features
- Recommendation: Split into separate commits
### Passed Checks
- ✅ All functions ≤ 50 lines
- ✅ All files within size limits
- ✅ No secrets detected
- ✅ Coverage at 82%
- ✅ Commit message format correct## Interactive Review: src/api/users.ts
### Function: createUser (lines 15-45)
**Observations**:
- 30 lines (within limit)
- Complexity: 6 (acceptable)
- Missing error handling for database failure
**Questions**:
1. Should we add explicit error handling for DB failures?
2. Should the validation schema be extracted to a separate file?
**Your Response**: [awaiting input]# .github/workflows/code-review.yml
name: Code Review Checks
on: [pull_request]
jobs:
review:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Function Length Check
run: |
# Check no function > 50 lines
# Implementation depends on language
- name: Security Scan
run: |
# Run secret detection
# Run input validation check
- name: Coverage Check
run: |
npm test -- --coverage
# Verify thresholds