diff --git a/.opencode/commands/review.md b/.opencode/commands/review.md index 23d7941ce7..70bd89ac90 100644 --- a/.opencode/commands/review.md +++ b/.opencode/commands/review.md @@ -3,60 +3,14 @@ Act as a senior software engineer and perform a thorough review. ## Instructions 1. **Determine what is being reviewed** from the provided context: - - **If it is a plan** (implementation plan, design document, task breakdown) → follow the **Plan Review** path below. - - **If it is code** (diff, PR, code change) → follow the **Code Review** path below. + - **If it is a plan** (implementation plan, design document, task breakdown) → load the **`plan-review`** skill. + - **If it is code** (diff, PR, code change) → load the **`code-review`** skill. ---- +2. Read `AGENTS.md` and follow its instructions for finding and reading all related testing documentation from memories before reviewing. -## Code Review Path +3. **Skip generated files, lockfile-only changes, and unrelated modifications** unless they introduce security risks. -1. Load the **`code-review-and-quality`** skill — it defines the five axes, core principles (DRY, KISS, YAGNI), severity taxonomy, and output format. -2. Read `AGENTS.md` and follow its instructions for finding and reading all related testing documentation from memories before reviewing the code. -3. Determine the diff or code to review from the provided context. -4. **Skip generated files, lockfile-only changes, and unrelated modifications** unless they introduce security risks. -5. Read the diff and the surrounding context for each changed file. -6. Review across all five axes: correctness, readability, architecture, security, performance. -7. Produce the review using the **Code Review Format** below. -8. For each finding: - - State the severity (Critical / High / Medium / Low / Suggestion) - - Identify the file and line - - Describe failure circumstances - - **For Critical/High**: Provide a concrete fix with a code snippet showing the corrected code - - **For Medium/Low**: Describe the fix clearly; code snippet optional - - If multiple approaches exist, briefly note trade-offs -9. **Perform a second review pass if the change is complex:** - - **Complex indicators**: Critical/High findings, multiple files (>5), architectural changes, security-sensitive code, >300 lines changed - - **Skip for simple changes**: Typo fixes, formatting, small bug fixes (<50 lines), single-file changes with no findings - - Second pass checks: - - Validate severity assignments: Are Critical/High findings truly blockers? - - Catch missed issues: Edge cases, error paths, test gaps overlooked in first pass - - Remove false positives: Discard findings that aren't real issues - - Verify fixes: Are the proposed solutions actually correct and complete? - ---- - -## Plan Review Path - -1. Load the **`plan-review`** skill — it defines the six axes, severity taxonomy, and output format. -2. Read the full plan from the provided context. -3. Review across all six axes: completeness, task quality, architecture & sequencing, risk coverage, actionability, and proposed code quality (if the plan includes implementation details). -4. Produce the review using the **Plan Review Format** below. -5. For each finding: - - State the severity (Critical / Required / Nit / Optional / FYI) - - Identify the section or task it refers to - - Describe the gap or problem - - **For Critical/Required**: Propose a concrete fix or addition - - **For Nit/Optional**: Describe the improvement; concrete text optional -6. **Perform a second review pass if the plan is complex:** - - **Complex indicators**: Critical findings, >10 tasks, migrations or breaking changes, security-sensitive features - - **Skip for simple plans**: 1–2 tasks, no risks, no code proposals - - Second pass checks: - - Validate severity assignments - - Catch missed gaps: edge cases, missing dependencies, unaddressed risks - - Remove false positives - - Verify proposed remedies are actionable - ---- +4. Follow the loaded skill's process and produce its output format. ## Strong Rules @@ -64,75 +18,8 @@ Act as a senior software engineer and perform a thorough review. 2. Do not modify any code and do not create a commit — this command only reviews. 3. Be specific and constructive. "This could be better" is not helpful — explain why and how. 4. Prioritize by impact. One structural issue outweighs ten nits. -5. Missing tests are an issue, not a suggestion. If tests are missing or inadequate for new functionality, report it as a severity-tagged finding in the findings sections below — High severity (code) or Required (plan) — never as a recommendation. +5. Missing tests are an issue, not a suggestion. Report as a severity-tagged finding — never as a recommendation. ## Context $ARGUMENTS - -## Expected Format — Code Review - -``` -## Review Summary -[1-2 sentences on what the change does and overall assessment] - -## Critical/High Findings - -### [Severity] file.ts:123 -**Issue**: [Description of the problem] -**Impact**: [What could go wrong if this is not fixed] -**Fix**: - -````[language] -// Current code -[problematic code] - -// Fixed code -[corrected code] -[Optional: note trade-offs if multiple approaches exist] -```` - -### [Severity] file.ts:456 -**Issue**: [Description of the problem] -**Impact**: [What could go wrong if this is not fixed] -**Fix**: [Clear description of the fix; code snippet if it clarifies] - -## Other Findings - -### [Severity] file.ts:789 -**Issue**: [Description] -**Impact**: [Minor consequence or risk] -**Fix**: [Clear description; code snippet optional] - -## Positive Observations -[2-3 specific things done well] - -## Verdict -[Approve / Request Changes / Needs Discussion] -[If Request Changes: list the must-fix items] -``` - -## Expected Format — Plan Review - -``` -## Review Summary -[1-2 sentences on the plan's goal and overall assessment] - -## Critical/Required Findings -### [Severity] [Section or Task N] -**Issue**: [Description of the gap or problem] -**Impact**: [What could go wrong during implementation] -**Proposed fix**: [Concrete addition or change to the plan] - -## Other Findings -### [Severity] [Section or Task N] -**Issue**: [Description] -**Proposed fix**: [Clear description; concrete text optional] - -## Strengths -[2-3 specific things done well in the plan] - -## Verdict -[Approve / Request Changes / Needs Discussion] -[If Request Changes: list the must-fix items] -``` diff --git a/.opencode/skills/code-review-and-quality/SKILL.md b/.opencode/skills/code-review/SKILL.md similarity index 99% rename from .opencode/skills/code-review-and-quality/SKILL.md rename to .opencode/skills/code-review/SKILL.md index a0f75e7f99..7f06d90b59 100644 --- a/.opencode/skills/code-review-and-quality/SKILL.md +++ b/.opencode/skills/code-review/SKILL.md @@ -1,5 +1,5 @@ --- -name: code-review-and-quality +name: code-review description: Conducts multi-axis code review. Use before merging any change. Use when reviewing code written by yourself, another agent, or a human. Use when you need to assess code quality across multiple dimensions before it enters the main branch. --- diff --git a/.opencode/skills/plan-review/SKILL.md b/.opencode/skills/plan-review/SKILL.md index 60b61c75e8..4386d701cc 100644 --- a/.opencode/skills/plan-review/SKILL.md +++ b/.opencode/skills/plan-review/SKILL.md @@ -87,7 +87,7 @@ Can an implementer actually execute this? ### 6. Proposed Code Quality *(when the plan includes implementation details)* -If the plan proposes code shapes, function signatures, data structures, or API designs, evaluate those proposals against `code-review-and-quality` criteria: +If the plan proposes code shapes, function signatures, data structures, or API designs, evaluate those proposals against `code-review` criteria: - **Correctness:** Do the proposed types/signatures handle edge cases (null, empty, boundaries)? - **Readability:** Are proposed names descriptive and consistent with project conventions? @@ -215,7 +215,7 @@ Check that the plan can actually confirm it worked: If the plan includes code snippets, types, or API designs: ``` -- Load code-review-and-quality skill for criteria +- Load code-review skill for criteria - Check proposed signatures for edge cases - Verify naming follows project conventions - Confirm abstractions follow existing patterns @@ -310,6 +310,6 @@ If the plan includes code snippets, types, or API designs: ## See Also - For producing plans, use the `planner` skill -- For reviewing implemented code, use `code-review-and-quality` — also the criteria source for axis 6 +- For reviewing implemented code, use `code-review` — also the criteria source for axis 6 - For security-specific concerns, see `security-and-hardening` - For testing strategy guidance, see `testing`