218 lines
8.5 KiB
Markdown
218 lines
8.5 KiB
Markdown
---
|
|
name: code-review
|
|
description: Review completed implementation work in the current repository. Reads PLAN.md and CODE_REVIEW.md from the active task, reviews the actual changed source files, appends a verdict to CODE_REVIEW.md, then archives both files as .log. On PASS, writes complete.log summarising the full loop history. If Required or Suggested issues are found (FAIL or WARN), writes a new PLAN.md and CODE_REVIEW.md stub so the loop continues immediately. Nit-only findings may still PASS.
|
|
---
|
|
|
|
# Code Review
|
|
|
|
## Purpose
|
|
|
|
Review the implementation phase of the plan-code-review loop:
|
|
|
|
```text
|
|
plan skill -> implementation -> code-review skill
|
|
^ |
|
|
+----- issues found: new PLAN.md
|
|
```
|
|
|
|
## Workflow Contract
|
|
|
|
Active work must live at `agent-task/{task_name}/PLAN.md` and `agent-task/{task_name}/CODE_REVIEW.md`. These paths are the state protocol shared with the plan skill. Do not adapt them per repository unless the whole loop contract is intentionally changed in both skills.
|
|
|
|
Directory states:
|
|
|
|
| State | Meaning |
|
|
|-------|---------|
|
|
| `PLAN.md` + `CODE_REVIEW.md` | Ready for review |
|
|
| `complete.log` + `*.log` files | Task complete (PASS) |
|
|
| Only `*.log` files (no `complete.log`) | Task terminated mid-loop or abandoned |
|
|
|
|
The implementing agent never archives or deletes active files; archiving is this skill's responsibility.
|
|
|
|
## Step 1 - Find Active Task
|
|
|
|
Glob `agent-task/*/CODE_REVIEW.md`:
|
|
|
|
| Result | Action |
|
|
|--------|--------|
|
|
| Exactly one | Review that task; `PLAN.md` is expected beside it |
|
|
| None | Nothing to review; stop and report |
|
|
| Multiple | List paths and ask which task to review |
|
|
|
|
## Step 2 - Load Context
|
|
|
|
Count `agent-task/{task_name}/code_review_*.log`:
|
|
|
|
- `0`: first review. Read `CODE_REVIEW.md`, `PLAN.md`, every planned source file, related tests, and files importing/imported by changed files up to 2 levels deep.
|
|
- `>=1`: follow-up review. Start with `git diff`, `git diff --cached`, and `git log --oneline -5`, then expand to related callers, implementers, tests, and any planned files missing from the diff.
|
|
|
|
The diff is the starting point, not the boundary. Follow behavior and API connections far enough to judge correctness.
|
|
|
|
## Step 3 - Pre-Review Checklist
|
|
|
|
Before writing the verdict:
|
|
|
|
- Compare actual source files against every planned checklist item.
|
|
- Grep renamed/removed symbols for stale references.
|
|
- Confirm every required test exists, name matches, and assertions are meaningful.
|
|
- Cross-check claimed verification output in `CODE_REVIEW.md` against actual code and project commands.
|
|
- For follow-up reviews, compare diff against the plan and scan for unplanned changes, debug prints, dead code, TODOs, formatting-only noise, and unrelated edits.
|
|
|
|
## Step 4 - Append Verdict
|
|
|
|
Append `코드리뷰 결과` to `CODE_REVIEW.md`.
|
|
|
|
Required fields:
|
|
|
|
- `종합 판정`: exactly `PASS`, `WARN`, or `FAIL`.
|
|
- `차원별 평가`: Pass/Warn/Fail for correctness, completeness, test coverage, API contract, code quality, plan deviation, verification trust.
|
|
- `발견된 문제`: `없음`, or bullets using `Required`, `Suggested`, or `Nit` with `file:line` and a concrete fix.
|
|
- `다음 단계`: keep only the matching PASS/WARN/FAIL line.
|
|
|
|
Severity semantics:
|
|
|
|
| Verdict | Meaning | Follow-up plan |
|
|
|---------|---------|----------------|
|
|
| `PASS` | No Required/Suggested issues. Nit-only findings may still PASS. | No |
|
|
| `WARN` | One or more Suggested issues, zero Required. | Yes |
|
|
| `FAIL` | One or more Required issues. | Yes |
|
|
|
|
Issue severity:
|
|
|
|
- `Required`: correctness, API contract, missing required test, or plan-completeness issue.
|
|
- `Suggested`: useful improvement that should enter the loop but does not block correctness.
|
|
- `Nit`: tiny cleanup; may be recorded without forcing WARN.
|
|
|
|
## Step 5 - Archive Active Files
|
|
|
|
Archive order is fixed:
|
|
|
|
1. Count existing `code_review_*.log` as `N`; rename `CODE_REVIEW.md` to `code_review_N.log`.
|
|
2. Count existing `plan_*.log` as `M`; rename `PLAN.md` to `plan_M.log`.
|
|
|
|
After archiving, neither active `.md` file remains unless Step 6 writes a follow-up.
|
|
|
|
## Step 6 - Post-Review Actions
|
|
|
|
For `PASS`, write `agent-task/{task_name}/complete.log` before reporting:
|
|
|
|
Required fields in `complete.log`:
|
|
- `완료 일시`: date completed.
|
|
- `요약`: one-line task description and loop count.
|
|
- `루프 이력`: table of plan/code_review log pairs with their verdict.
|
|
- `최종 리뷰 요약`: bullet list of what was implemented.
|
|
- `잔여 Nit`: any Nit-only findings recorded but not acted on (omit section if none).
|
|
|
|
Then report:
|
|
|
|
- Verdict.
|
|
- Archive filenames.
|
|
- `complete.log` written; task complete.
|
|
|
|
For `WARN` or `FAIL`, write a new `PLAN.md` and `CODE_REVIEW.md` stub using the plan skill format:
|
|
|
|
- New plan number is the count of `plan_*.log` after archive.
|
|
- Header tag is `REVIEW_<PARENT_TAG>`.
|
|
- `FAIL`: one plan item per Required issue.
|
|
- `WARN`: one grouped plan item for Suggested issues, plus related Nit issues if useful.
|
|
- Each plan item needs problem, solution with before/after when non-trivial, checklist, test decision, intermediate verification.
|
|
|
|
`CODE_REVIEW.md` stub template (fill `{…}` placeholders; everything else is fixed and must not be changed by the implementing agent):
|
|
|
|
```markdown
|
|
<!-- task={task_name} plan={N} tag={TAG} -->
|
|
|
|
# Code Review Reference - {TAG}
|
|
|
|
## 개요
|
|
|
|
date={YYYY-MM-DD}
|
|
task={task_name}, plan={N}, tag={TAG}
|
|
|
|
## 이 파일을 읽는 리뷰 에이전트에게
|
|
|
|
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
|
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
|
|
|
1. `CODE_REVIEW.md` → `code_review_N.log` (N = 기존 code_review_*.log 수)
|
|
2. `PLAN.md` → `plan_M.log` (M = 기존 plan_*.log 수)
|
|
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.
|
|
|
|
---
|
|
|
|
## 구현 항목별 완료 여부
|
|
|
|
| 항목 | 완료 여부 |
|
|
|------|---------|
|
|
| [{TAG}-1] {item description} | [ ] |
|
|
| [{TAG}-2] {item description} | [ ] |
|
|
|
|
## 계획 대비 변경 사항
|
|
|
|
_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._
|
|
|
|
## 주요 설계 결정
|
|
|
|
_구현 에이전트가 주요 설계 결정 사항을 기록한다._
|
|
|
|
## 리뷰어를 위한 체크포인트
|
|
|
|
{pre-filled from plan — one bullet per review focus area}
|
|
|
|
## 검증 결과
|
|
|
|
_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._
|
|
|
|
### {TAG}-1 중간 검증
|
|
```
|
|
$ {verification command from plan}
|
|
(output)
|
|
```
|
|
|
|
### 최종 검증
|
|
```
|
|
$ {final verification command from plan}
|
|
(output)
|
|
```
|
|
```
|
|
|
|
Sections and their ownership:
|
|
|
|
| 섹션 | 소유자 | 설명 |
|
|
|------|--------|------|
|
|
| 헤더 주석, 개요(date/task/plan/tag), 리뷰 에이전트 지시 | 스텁 생성 시 고정 | 구현 에이전트가 수정하지 않음 |
|
|
| 구현 항목별 완료 여부 (항목명) | 스텁 생성 시 고정 | `[ ]` → `[x]` 체크만 구현 에이전트가 수행 |
|
|
| 계획 대비 변경 사항, 주요 설계 결정 | 구현 에이전트가 채움 | placeholder 텍스트를 실제 내용으로 교체 |
|
|
| 리뷰어를 위한 체크포인트 | 스텁 생성 시 고정 | 계획에서 추출한 리뷰 포인트 |
|
|
| 검증 결과 (섹션 제목 + 명령) | 스텁 생성 시 고정 | 실행 출력만 구현 에이전트가 채움 |
|
|
| 코드리뷰 결과 | 리뷰 에이전트가 append | 스텁에 포함하지 않음 |
|
|
|
|
Report Required/Suggested counts, archive names, and the new plan path.
|
|
|
|
## Review Dimensions
|
|
|
|
| Dimension | Check |
|
|
|-----------|-------|
|
|
| Correctness | Logic, edge cases, concurrency, errors |
|
|
| Completeness | All planned checklist items done |
|
|
| Test coverage | Required tests present and meaningful |
|
|
| API contract | Call sites, compatibility, docs |
|
|
| Code quality | No debug prints, dead code, leftover TODOs |
|
|
| Plan deviation | Deviations justified, no unrelated risk |
|
|
| Verification trust | Reported output matches actual code |
|
|
|
|
## Quality Rules
|
|
|
|
- Lead with findings; use specific `file:line`.
|
|
- Provide a concrete fix for every Required issue.
|
|
- Name exact stale symbols or missing tests.
|
|
- Do not write vague praise or style opinions without a rule.
|
|
- Every dimension gets Pass/Warn/Fail.
|
|
|
|
## Final Checklist
|
|
|
|
- `code_review_N.log` exists with verdict appended.
|
|
- `plan_M.log` exists.
|
|
- No active `.md` files remain after PASS.
|
|
- PASS: `complete.log` written with loop history, implementation summary, and residual Nits.
|
|
- WARN/FAIL: new active `PLAN.md` and `CODE_REVIEW.md` created with matching headers; no `complete.log`.
|