forge-code-review

Forge playbook — Code review method

Self-improvement substrate (wp-skill-evals, 2026-07-31): before applying this skill, read learnings.md in this skill's own folder and honor its corrections. After a run that produced a genuine correction (an owner fix, a false assumption caught, a preference stated), append it to learnings.md with a date and real evidence — never invent a lesson that didn't happen.

Do not duplicate ECC skills — defer to: code-review-excellence (the deep per-language review reference: React/Vue/Angular/Svelte/Rust/TypeScript/Java/PHP/Python/Django/Go/C#/Kotlin/NestJS/C/C++). This file is the Forge-specific orchestration wrapper: the fixed review order, the severity scale, and how a finding becomes a rework item in the Forge pipeline.

Hard rules

• Review the real diff/files directly — never review a self-report of the diff, and never approve on the strength of "tests pass" alone without reading the actual change.
• Every finding is specific and actionable (file + line/symbol + what's wrong + the fix), never a vague "looks off."
• CRITICAL/HIGH findings block completion; they are not "nice to have later."
• Feedback targets the code, not the person; explain the reasoning, don't just assert a preference.
• A review is not a rewrite-to-taste pass — flag genuine correctness/security/maintainability issues, not personal style preferences that don't match a real project convention.

Review order (fixed sequence — don't skip ahead to style)

1. Correctness — does the logic actually do what it claims; are edge cases the happy path hides named, not assumed away.
2. Security — secrets, injected input, auth/authz boundaries, unsafe deserialization, path traversal, anything on the mandatory security checklist (~/.claude/rules/ecc/common/security.md) for security-sensitive changes.
3. Tests — does new behavior have a real test; do existing tests still pass; is coverage of the changed logic genuine (not just line-count coverage).
4. Simplicity — is this the simplest correct solution, or is there unnecessary abstraction/complexity for what the task needed (YAGNI).
5. Performance — any obvious N+1 queries, unbounded loops/queries, or missing pagination/caching on a hot path.
6. Style — naming, formatting, project convention consistency — reviewed last, and never blocking on its own unless it actively harms readability.