refactor(projman): extract skills and consolidate commands
Major refactoring of projman plugin architecture: Skills Extraction (17 new files): - Extracted reusable knowledge from commands and agents into skills/ - branch-security, dependency-management, git-workflow, input-detection - issue-conventions, lessons-learned, mcp-tools-reference, planning-workflow - progress-tracking, repo-validation, review-checklist, runaway-detection - setup-workflows, sprint-approval, task-sizing, test-standards, wiki-conventions Command Consolidation (17 → 12 commands): - /setup: consolidates initial-setup, project-init, project-sync (--full/--quick/--sync) - /debug: consolidates debug-report, debug-review (report/review modes) - /test: consolidates test-check, test-gen (run/gen modes) - /sprint-status: absorbs sprint-diagram via --diagram flag Architecture Cleanup: - Remove plugin-level mcp-servers/ symlinks (6 plugins) - Remove plugin README.md files (12 files, ~2000 lines) - Update all documentation to reflect new command structure - Fix documentation drift in CONFIGURATION.md, COMMANDS-CHEATSHEET.md Commands are now thin dispatchers (~20-50 lines) that reference skills. Agents reference skills for domain knowledge instead of inline content. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
149
plugins/projman/skills/review-checklist.md
Normal file
149
plugins/projman/skills/review-checklist.md
Normal file
@@ -0,0 +1,149 @@
|
||||
---
|
||||
name: review-checklist
|
||||
description: Code review criteria and severity classification
|
||||
---
|
||||
|
||||
# Review Checklist
|
||||
|
||||
## Purpose
|
||||
|
||||
Defines code review criteria, severity classification, and output format.
|
||||
|
||||
## When to Use
|
||||
|
||||
- **Code Reviewer agent**: During pre-sprint-close review
|
||||
- **Commands**: `/review`
|
||||
|
||||
---
|
||||
|
||||
## Severity Classification
|
||||
|
||||
### Critical (Must Fix Before Close)
|
||||
|
||||
Security issues, broken functionality, data loss risks:
|
||||
|
||||
- Hardcoded credentials or API keys
|
||||
- SQL injection vulnerabilities
|
||||
- Missing authentication/authorization checks
|
||||
- Unhandled errors that could crash the application
|
||||
- Data loss or corruption risks
|
||||
- Broken core functionality
|
||||
|
||||
### Warning (Should Fix)
|
||||
|
||||
Technical debt that will cause problems soon:
|
||||
|
||||
- TODO/FIXME comments left unresolved
|
||||
- Debug statements (console.log, print) in production code
|
||||
- Functions over 50 lines (complexity smell)
|
||||
- Deeply nested conditionals (>3 levels)
|
||||
- Bare except/catch blocks
|
||||
- Ignored errors
|
||||
- Missing error handling
|
||||
|
||||
### Recommendation (Future Sprint)
|
||||
|
||||
Improvements that can wait:
|
||||
|
||||
- Missing docstrings on public functions
|
||||
- Minor code duplication
|
||||
- Commented-out code blocks
|
||||
- Variable naming improvements
|
||||
- Minor refactoring opportunities
|
||||
|
||||
---
|
||||
|
||||
## Review Patterns by Language
|
||||
|
||||
### Python
|
||||
| Look For | Severity |
|
||||
|----------|----------|
|
||||
| Bare `except:` | Warning |
|
||||
| `print()` statements | Warning |
|
||||
| `# TODO` | Warning |
|
||||
| Missing type hints on public APIs | Recommendation |
|
||||
| `eval()`, `exec()` | Critical |
|
||||
| SQL string formatting | Critical |
|
||||
| `verify=False` in requests | Critical |
|
||||
|
||||
### JavaScript/TypeScript
|
||||
| Look For | Severity |
|
||||
|----------|----------|
|
||||
| `console.log` | Warning |
|
||||
| `// TODO` | Warning |
|
||||
| `any` type abuse | Warning |
|
||||
| Missing error boundaries | Warning |
|
||||
| `eval()` | Critical |
|
||||
| `innerHTML` with user input | Critical |
|
||||
| Unescaped user input | Critical |
|
||||
|
||||
### Go
|
||||
| Look For | Severity |
|
||||
|----------|----------|
|
||||
| `// TODO` | Warning |
|
||||
| Ignored errors (`_`) | Warning |
|
||||
| Missing error returns | Warning |
|
||||
| SQL concatenation | Critical |
|
||||
| Missing input validation | Warning |
|
||||
|
||||
### Rust
|
||||
| Look For | Severity |
|
||||
|----------|----------|
|
||||
| `// TODO` | Warning |
|
||||
| `unwrap()` chains | Warning |
|
||||
| `unsafe` blocks without justification | Warning |
|
||||
| Unchecked `unwrap()` on user input | Critical |
|
||||
|
||||
---
|
||||
|
||||
## What NOT to Review
|
||||
|
||||
- Style issues (assume formatters handle this)
|
||||
- Architectural rewrites mid-sprint
|
||||
- Issues in unchanged code (unless directly impacted)
|
||||
- Bikeshedding on naming preferences
|
||||
|
||||
---
|
||||
|
||||
## Output Template
|
||||
|
||||
```
|
||||
## Code Review Summary
|
||||
|
||||
**Scope**: [X files from sprint/last N commits]
|
||||
**Verdict**: [READY FOR CLOSE / NEEDS ATTENTION / BLOCKED]
|
||||
|
||||
### Critical (Must Fix)
|
||||
- `src/auth.py:45` - Hardcoded API key in source code
|
||||
- `src/db.py:123` - SQL injection vulnerability
|
||||
|
||||
### Warnings (Should Fix)
|
||||
- `src/utils.js:123` - console.log left in production code
|
||||
- `src/handler.py:67` - Bare except block swallows all errors
|
||||
|
||||
### Recommendations (Future Sprint)
|
||||
- `src/api.ts:89` - Function exceeds 50 lines, consider splitting
|
||||
|
||||
### Clean Files
|
||||
- src/models.py
|
||||
- src/tests/test_auth.py
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Verdict Criteria
|
||||
|
||||
| Verdict | Criteria |
|
||||
|---------|----------|
|
||||
| **READY FOR CLOSE** | No Critical, few/no Warnings |
|
||||
| **NEEDS ATTENTION** | No Critical, has Warnings that should be addressed |
|
||||
| **BLOCKED** | Has Critical issues that must be fixed |
|
||||
|
||||
---
|
||||
|
||||
## Integration with Sprint
|
||||
|
||||
When sprint context is available:
|
||||
- Reference the sprint's issue list
|
||||
- Create follow-up issues for non-critical findings
|
||||
- Tag findings with appropriate labels from taxonomy
|
||||
Reference in New Issue
Block a user