Structured code review of git changes with senior engineer rigor. Checks SOLID violations, security risks, race conditions, error handling, performance, boundary bugs, and removal candidates. Outputs severity-classified findings (P0–P3) with actionable fixes. Don't use it for general coding tasks, non-git file editing, or writing new code from scratch.
Structured review of git changes through a senior engineer lens. Detects SOLID violations, security risks, race conditions, performance regressions, and boundary bugs. Outputs severity-classified findings with actionable fixes. Review-first by default — no changes until you confirm.
This skill follows the Reviewer pattern: checklist-driven analysis with severity classification.
references/ and are loaded on demand at each workflow stepRun these commands to understand what you're reviewing:
git status -sb
git ls-files --others --exclude-standard
git diff --stat
git diff
If the user specified a commit range or branch comparison, use that instead:
git diff <base>...<head> --stat
git diff <base>...<head>
Treat untracked files returned by git ls-files --others --exclude-standard as full-file additions when they fall inside the requested review scope.
Use rg or grep to find related modules, usages, and contracts when context is needed.
Decision table for diff source:
| Situation | What to review | Command |
|---|---|---|
| Unstaged changes exist | Unstaged diff | git diff |
| Only staged changes | Staged diff | git diff --cached |
| Untracked files exist | Review as new-file additions | git ls-files --others --exclude-standard + full file read |
| User specifies commit range | Range diff | git diff <base>...<head> |
| User specifies branch | Branch diff | git diff main...<branch> |
| No changes at all | Ask user | Inform and ask for commit range or branch |
| Not a git repo | Stop | Inform user this skill requires a git repository |
Large diff (>500 lines): Summarize by file first (git diff --stat), then review in batches grouped by module/feature area. Don't try to review everything in one pass.
Mixed concerns: Group findings by logical feature, not file order.
Load references/solid-checklist.md and check each item against the diff.
Focus areas:
When proposing a refactor, explain why it improves cohesion/coupling. If the refactor is non-trivial, propose an incremental plan rather than a rewrite.
Load references/removal-plan.md for the template.
Identify code that is unused, redundant, or feature-flagged off. Distinguish:
Load references/security-checklist.md and check each category.
Priority targets: auth boundaries, data writes, network calls, user input handling, crypto usage, concurrent access patterns.
Call out both exploitability and impact for each finding.
Load references/code-quality-checklist.md and check each category.
Priority targets: error handling gaps, N+1 queries, unbounded memory, null/undefined handling, off-by-one errors, async error propagation.
Flag issues that may cause silent failures or production incidents.
## Code Review Summary
**Files reviewed**: X files, Y lines changed
**Overall assessment**: [APPROVE / REQUEST_CHANGES / COMMENT]
---
### P0 — Critical
(none or list)
### P1 — High
1. **[file:line]** Brief title
- Description of issue
- Suggested fix
### P2 — Medium
(continue numbering)
### P3 — Low
...
---
### Removal/Iteration Plan
(if applicable)
### Additional Suggestions
(optional, not blocking)
For file-specific inline comments, use:
::code-comment{file="path/to/file.ts" line="42" severity="P1"}
Description and suggested fix.
::
Clean review: If no issues found, explicitly state what was checked and any areas not covered (e.g., "Did not verify database migrations"). Note residual risks.
⚠️ REQUIRED: Do NOT implement any changes until user explicitly confirms.
---
## Next Steps
Found X issues (P0: _, P1: _, P2: _, P3: _).
**How would you like to proceed?**
1. **Fix all** — implement all suggested fixes
2. **Fix P0/P1 only** — address critical and high priority
3. **Fix specific items** — tell me which issues to fix
4. **No changes** — review complete, no implementation needed
| Severity | Name | Action | Blocks merge? |
|---|---|---|---|
| P0 | Critical | Security vuln, data loss, correctness bug | Yes |
| P1 | High | Logic error, major SOLID violation, perf regression | Should fix first |
| P2 | Medium | Code smell, maintainability, minor SOLID violation | Fix or create follow-up |
| P3 | Low | Style, naming, minor suggestion | Optional |
| Review phase | Reference file | Focus |
|---|---|---|
| SOLID | references/solid-checklist.md | Architecture smells, coupling |
| Removal | references/removal-plan.md | Dead code, feature flags |
| Security | references/security-checklist.md | Injection, auth, race conditions |
| Quality | references/code-quality-checklist.md | Errors, perf, boundaries |
| Coding Guidelines | ~/.openclaw/shared/coding-guidelines.md | Anti-Pattern: 假设隐藏、过度复杂、非手术刀改动、弱目标、命令式指令、概念错误、附和倾向、跳过人类审查 |
| Mistake | Why It Fails | Better Approach |
|---|---|---|
| Reviewing without reading surrounding code | Misses context, false positives | Use rg to find callers and contracts before flagging |
| Flagging style issues as P1 | Noise drowns real issues | Reserve P1+ for correctness, security, logic errors |
| Proposing large rewrites in review | Unrealistic, blocks merge | Propose incremental plan with concrete first step |
| Implementing fixes without asking | User may disagree with approach | Always present findings first, wait for confirmation |
| Reviewing entire file instead of diff | Wastes time, scope creep | Focus on changed lines and their immediate context |
Inform the user: "This directory is not a git repository. Code review requires git context to identify changes. Please run from a git repo or specify files to review manually."
Ask the user what they want reviewed: staged changes (git diff --cached), a specific commit range, or a branch comparison. Don't silently do nothing.
If git diff or git status fails (permissions, corrupt repo, etc.), report the error and suggest the user check their git state. Don't guess.
If the diff touches 20+ files across unrelated modules, ask the user if they want a full review or want to focus on specific areas. Don't silently skip files.
If you couldn't cover all phases (e.g., skipped security due to context limits), explicitly state what was and wasn't reviewed. Never claim a complete review when it wasn't.
git diff shows unstaged changes only. If user staged everything, you'll see an empty diff and think there's nothing to review. Always check git diff --cached as fallback..github/CODEOWNERS or review checklist, mention it — the user may have team-specific review requirements you should incorporate.review current changes or review this diff.scaffold, mvp, production-ready, and integrated.integrated.failed or blocked instead of claiming completion.| File | Purpose |
|---|---|
references/solid-checklist.md | SOLID smell prompts, code smell table, refactor heuristics |
references/security-checklist.md | Input safety, auth, JWT, secrets, CORS, runtime risks, race conditions, data integrity |
references/code-quality-checklist.md | Error handling, performance/caching, boundary conditions |
references/removal-plan.md | Template for deletion candidates and deferred removal plans |