skills: add code-review and implement skill files
- Add skills/code-review/SKILL.md - Add skills/implement/SKILL.md
This commit is contained in:
parent
840df6a63d
commit
24adec1c1d
2 changed files with 446 additions and 0 deletions
193
skills/code-review/SKILL.md
Normal file
193
skills/code-review/SKILL.md
Normal file
|
|
@ -0,0 +1,193 @@
|
|||
---
|
||||
name: code-review
|
||||
description: Review completed implementation work for toki_socket. 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. If Required issues are found, writes a new PLAN.md and CODE_REVIEW.md stub so the loop continues immediately.
|
||||
---
|
||||
|
||||
# Code Review
|
||||
|
||||
## Overview
|
||||
|
||||
This skill is the third step in the implement → code loop:
|
||||
|
||||
```
|
||||
implement skill → implementing agent → code-review skill (this)
|
||||
↑ |
|
||||
└──────── (issues found) ── new PLAN.md ─┘
|
||||
```
|
||||
|
||||
**Directory state signals:**
|
||||
|
||||
| State | Meaning |
|
||||
|-------|---------|
|
||||
| `PLAN.md` + `CODE_REVIEW.md` both present | Ready for code review ← run this skill |
|
||||
| Only `*.log` files | Task complete |
|
||||
|
||||
Note: `PLAN.md` and `CODE_REVIEW.md` are always present together. The implementing agent does not rename or delete either file — archiving is this skill's responsibility.
|
||||
|
||||
---
|
||||
|
||||
## Step 1 — Find the Active Task
|
||||
|
||||
1. Glob `tasks/*/CODE_REVIEW.md`.
|
||||
2. If exactly one exists: that is the active task. Record `task_name`. Confirm `PLAN.md` is also present in the same directory.
|
||||
3. If no `CODE_REVIEW.md` found: nothing to review. Stop and report.
|
||||
4. If multiple: list them and ask the user which to review. Stop.
|
||||
|
||||
---
|
||||
|
||||
## Step 2 — Load Context
|
||||
|
||||
Read all of the following before evaluating anything:
|
||||
|
||||
1. `tasks/{task_name}/CODE_REVIEW.md` — implementation report filled by the implementing agent.
|
||||
2. `tasks/{task_name}/PLAN.md` — the plan the implementation followed.
|
||||
3. Every source file listed in the plan's "수정 파일 및 체크리스트" — read the actual current file.
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Pre-Review Checklist
|
||||
|
||||
Complete every item before writing the verdict.
|
||||
|
||||
- [ ] Cross-check every plan checkbox against the actual source file — was the change applied?
|
||||
- [ ] Grep for every renamed or removed symbol — confirm no stale references anywhere.
|
||||
- [ ] For every new test listed in the plan: confirm it exists, has the right name, and the assertion is non-trivial (not just "no exception thrown").
|
||||
- [ ] Check for regressions: read tests covering adjacent behavior not mentioned in the plan.
|
||||
- [ ] Cross-check the claimed `dart analyze` / `go test` output against the actual code.
|
||||
- [ ] Check for code quality issues: debug prints, dead code, leftover TODOs.
|
||||
|
||||
---
|
||||
|
||||
## Step 4 — Append Verdict to CODE_REVIEW.md
|
||||
|
||||
Append the following section to the existing `CODE_REVIEW.md`. Every field is required.
|
||||
|
||||
```markdown
|
||||
---
|
||||
|
||||
## 코드리뷰 결과
|
||||
|
||||
### 종합 판정
|
||||
|
||||
**[ PASS | WARN | FAIL ]**
|
||||
|
||||
> <1–2 sentences summarising the overall assessment.>
|
||||
|
||||
### 차원별 평가
|
||||
|
||||
| 차원 | 판정 | 비고 |
|
||||
|------|------|------|
|
||||
| 정확성 (Correctness) | Pass/Warn/Fail | |
|
||||
| 완성도 (Completeness) | Pass/Warn/Fail | |
|
||||
| 테스트 커버리지 | Pass/Warn/Fail | |
|
||||
| API 계약 | Pass/Warn/Fail | |
|
||||
| 코드 품질 | Pass/Warn/Fail | |
|
||||
| 계획 대비 이탈 | Pass/Warn/Fail | |
|
||||
| 검증 신뢰도 | Pass/Warn/Fail | |
|
||||
|
||||
### 발견된 문제
|
||||
|
||||
<!-- 없으면 "없음" 한 줄 -->
|
||||
- **[Required|Suggested|Nit]** `<file>:<line>` — <problem>. Fix: <concrete fix>.
|
||||
|
||||
### 다음 단계
|
||||
|
||||
<!-- 해당하는 하나만 남기고 나머지 줄은 삭제 -->
|
||||
**[PASS]** 추가 작업 불필요. 아카이브 완료.
|
||||
**[FIX]** Required 문제 N건. 새 PLAN.md 작성 완료. 구현 에이전트는 PLAN.md를 읽고 작업 시작.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Step 5 — Archive Both Files
|
||||
|
||||
Archive order: CODE_REVIEW.md first, then PLAN.md.
|
||||
|
||||
**Archive CODE_REVIEW.md:**
|
||||
- Count existing `tasks/{task_name}/code_review_*.log` → that count is N.
|
||||
- Rename `tasks/{task_name}/CODE_REVIEW.md` → `tasks/{task_name}/code_review_N.log`.
|
||||
|
||||
**Archive PLAN.md:**
|
||||
- Count existing `tasks/{task_name}/plan_*.log` → that count is M.
|
||||
- Rename `tasks/{task_name}/PLAN.md` → `tasks/{task_name}/plan_M.log`.
|
||||
|
||||
After this step neither `PLAN.md` nor `CODE_REVIEW.md` exists.
|
||||
|
||||
---
|
||||
|
||||
## Step 6 — Post-Review Actions
|
||||
|
||||
### If verdict is PASS (zero Required issues)
|
||||
|
||||
No further files needed. Report to the user:
|
||||
- Verdict: PASS
|
||||
- Archives: `code_review_N.log`, `plan_M.log`
|
||||
- Task status: complete (only `.log` files remain in `tasks/{task_name}/`)
|
||||
|
||||
### If verdict is WARN or FAIL (one or more Required issues)
|
||||
|
||||
Write new files for the fix round:
|
||||
|
||||
**Write `tasks/{task_name}/PLAN.md`** following the implement skill format exactly:
|
||||
- Count `tasks/{task_name}/plan_*.log` files **after** the archive above → that count is the new plan's N.
|
||||
- Header: `<!-- task={task_name} plan=N tag=REVIEW_<PARENT_TAG> -->`
|
||||
- Include the "구현 에이전트에게" section.
|
||||
- One plan item per Required issue:
|
||||
- **문제**: quote the specific `file:line` from the review finding.
|
||||
- **해결 방법**: before/after code block — read the actual source for current state.
|
||||
- **수정 파일 및 체크리스트**: exhaustive checkbox list.
|
||||
- **테스트 작성**: decision + justification.
|
||||
- **중간 검증**: runnable command.
|
||||
- Suggested/Nit issues may be grouped into one optional item.
|
||||
- Include 수정 파일 요약 table.
|
||||
- Include 최종 검증 section.
|
||||
|
||||
**Write `tasks/{task_name}/CODE_REVIEW.md`** stub:
|
||||
- Header: `<!-- task={task_name} plan=N tag=REVIEW_<PARENT_TAG> -->` (same N as new PLAN.md)
|
||||
- Completion table listing every new plan item with `[ ]`.
|
||||
- Empty sections: 계획 대비 변경 사항, 주요 설계 결정, 리뷰어 체크포인트, 검증 결과.
|
||||
|
||||
Report to the user:
|
||||
- Verdict: WARN/FAIL, N Required issues
|
||||
- Archives: `code_review_N.log`, `plan_M.log`
|
||||
- New plan: `tasks/{task_name}/PLAN.md` ready for implementing agent
|
||||
|
||||
---
|
||||
|
||||
## Review Dimensions Reference
|
||||
|
||||
| Dimension | What to check |
|
||||
|-----------|---------------|
|
||||
| **정확성** | Logic errors, off-by-ones, wrong types, missed edge cases |
|
||||
| **완성도** | All plan checkboxes done; none silently skipped |
|
||||
| **테스트 커버리지** | Required tests present; assertions are meaningful |
|
||||
| **API 계약** | All call sites updated; breaking changes documented |
|
||||
| **코드 품질** | No debug prints, dead code, leftover TODOs |
|
||||
| **계획 대비 이탈** | Deviations justified; no new unreviewed risk |
|
||||
| **검증 신뢰도** | Reported output consistent with actual code |
|
||||
|
||||
---
|
||||
|
||||
## Quality Rules
|
||||
|
||||
**Do write:**
|
||||
- Specific `file:line` for every issue.
|
||||
- A concrete fix for every Required issue.
|
||||
- The exact symbol name when Grep finds stale references.
|
||||
- The exact test name when a required test is missing.
|
||||
|
||||
**Do not write:**
|
||||
- "This looks fine" without having read the code.
|
||||
- Style warnings with no linter rule backing them.
|
||||
- Vague verdicts — every dimension gets Pass/Warn/Fail.
|
||||
|
||||
---
|
||||
|
||||
## Final Verification Checklist
|
||||
|
||||
1. `code_review_N.log` exists with verdict section appended.
|
||||
2. `plan_M.log` exists (was PLAN.md).
|
||||
3. Neither `CODE_REVIEW.md` nor `PLAN.md` exists as `.md` in the task directory.
|
||||
4. If PASS: only `.log` files remain in `tasks/{task_name}/`.
|
||||
5. If FAIL: new `PLAN.md` exists, header `plan=N` matches count of `plan_*.log` after archiving, covers every Required issue with full sub-sections.
|
||||
6. If FAIL: new `CODE_REVIEW.md` stub exists, header `plan=N` matches new PLAN.md, completion table lists every item.
|
||||
253
skills/implement/SKILL.md
Normal file
253
skills/implement/SKILL.md
Normal file
|
|
@ -0,0 +1,253 @@
|
|||
---
|
||||
name: implement
|
||||
description: Analyze the codebase and write a detailed PLAN.md for toki_socket improvements. Also writes the CODE_REVIEW.md stub that the implementing agent will fill in after coding. Use for any new feature, refactor, bug fix, or follow-up fix. A separate agent reads PLAN.md and does the actual coding. The code-review skill archives both files after review.
|
||||
---
|
||||
|
||||
# Implement
|
||||
|
||||
## Overview
|
||||
|
||||
This skill produces two files that drive the implementation loop:
|
||||
|
||||
```
|
||||
implement skill (this) → PLAN.md + CODE_REVIEW.md stub
|
||||
↓
|
||||
Separate implementing agent → reads PLAN.md, codes, fills CODE_REVIEW.md
|
||||
↓
|
||||
code-review skill → reads both, archives both, loops if needed
|
||||
```
|
||||
|
||||
All files live under `tasks/{task_name}/`. Directory state at any point:
|
||||
|
||||
| State | Meaning |
|
||||
|-------|---------|
|
||||
| `PLAN.md` only (no `CODE_REVIEW.md`) | Should not occur — implement skill always writes both |
|
||||
| `PLAN.md` + `CODE_REVIEW.md` (stub, unfilled) | Implementing agent is working |
|
||||
| `PLAN.md` + `CODE_REVIEW.md` (filled) | Ready for code-review skill |
|
||||
| Only `*.log` files | Task complete |
|
||||
|
||||
---
|
||||
|
||||
## Step 1 — Determine Task
|
||||
|
||||
### New task
|
||||
|
||||
1. Derive `task_name`: short snake_case from the user request (e.g., `dart_api_refactor`, `go_base_client`).
|
||||
2. Create `tasks/{task_name}/` if it does not exist.
|
||||
|
||||
### Continuing an existing task (re-plan triggered by code-review)
|
||||
|
||||
1. Glob `tasks/*/PLAN.md` — if exactly one exists, that is the active task. Use its directory.
|
||||
2. If none but `tasks/*/CODE_REVIEW.md` exists: the task is waiting for `code-review`, not this skill. Stop and report.
|
||||
3. If the user names the task explicitly, use that name regardless.
|
||||
|
||||
---
|
||||
|
||||
## Step 2 — Analyze Codebase
|
||||
|
||||
Complete every item before writing a single line. Do not skip.
|
||||
|
||||
- [ ] Read every source file the change will touch — the whole file, not just the header.
|
||||
- [ ] Read every test file that exercises the changed code.
|
||||
- [ ] For each changed behavior, check whether an existing test covers it. Note uncovered gaps.
|
||||
- [ ] Grep for every symbol that will be renamed or removed — find all call sites and import chains.
|
||||
- [ ] Check `pubspec.yaml` / `go.mod` for existing dependencies before adding new ones.
|
||||
- [ ] Anticipate compile errors: missing `@override`, type mismatches, broken imports.
|
||||
- [ ] Confirm the final verification command works in this project layout.
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Archive Previous Files (if any)
|
||||
|
||||
Before writing new files, archive any active files from a previous round:
|
||||
|
||||
**Archive PLAN.md:**
|
||||
- Count existing `tasks/{task_name}/plan_*.log` → that count is N.
|
||||
- If `PLAN.md` exists, rename it to `plan_N.log`.
|
||||
|
||||
**Archive CODE_REVIEW.md:**
|
||||
- Count existing `tasks/{task_name}/code_review_*.log` → that count is M.
|
||||
- If `CODE_REVIEW.md` exists, rename it to `code_review_M.log`.
|
||||
|
||||
---
|
||||
|
||||
## Step 4 — Write PLAN.md
|
||||
|
||||
Write `tasks/{task_name}/PLAN.md`:
|
||||
|
||||
```markdown
|
||||
# <Title>
|
||||
|
||||
<!-- task={task_name} plan=N tag=<TAG> -->
|
||||
|
||||
## 이 파일을 읽는 구현 에이전트에게
|
||||
|
||||
- 각 항목의 체크리스트를 순서대로 완료한다.
|
||||
- 항목 완료마다 **중간 검증** 커맨드를 실행하고 통과를 확인한다.
|
||||
- 모든 항목 완료 후 **최종 검증** 커맨드를 실행한다.
|
||||
- 작업이 끝나면 `CODE_REVIEW.md`의 각 섹션을 실제 구현 내용으로 채운다:
|
||||
- 구현 항목별 완료 여부 체크
|
||||
- 계획 대비 변경 사항 (없으면 "계획서 그대로 구현")
|
||||
- 주요 설계 결정
|
||||
- 리뷰어를 위한 체크포인트
|
||||
- 검증 결과 (실제 실행한 명령과 출력 붙여넣기)
|
||||
|
||||
## 배경
|
||||
|
||||
<2–4 sentences: why this change is needed, what problem it solves.>
|
||||
|
||||
---
|
||||
|
||||
### [<TAG>-1] <Short title>
|
||||
|
||||
**문제**
|
||||
|
||||
<Concrete description. Quote actual code with file:line reference.>
|
||||
|
||||
**해결 방법**
|
||||
|
||||
<Exact solution with design rationale. Before/after code block for every non-trivial change.>
|
||||
|
||||
\```<language>
|
||||
// Before — <file>:<line>
|
||||
<old code>
|
||||
|
||||
// After
|
||||
<new code>
|
||||
\```
|
||||
|
||||
**수정 파일 및 체크리스트**
|
||||
|
||||
- [ ] `<path/to/file>` — <one-line summary>
|
||||
- [ ] <specific change, e.g. "line 34: rename dispose() to close()">
|
||||
- [ ] <specific change 2>
|
||||
|
||||
**테스트 작성**
|
||||
|
||||
| 상황 | 요구 사항 |
|
||||
|------|-----------|
|
||||
| 버그 수정 | 회귀 테스트 필수 |
|
||||
| 새 공개 API | 정상 동작 + 경계 조건 테스트 필수 |
|
||||
| API 이름 변경 | 기존 테스트 호출부 수정으로 충분 |
|
||||
| 내부 리팩터링 | 기존 테스트 통과로 충분 |
|
||||
| 동시성 로직 | 경쟁 조건 테스트 추가 권장 |
|
||||
|
||||
테스트가 필요하면: 파일 경로, 테스트 이름, 검증 목표, 필요한 픽스처를 명시.
|
||||
불필요하면: 이유 한 줄 기재 (이 섹션 자체는 생략 금지).
|
||||
|
||||
**중간 검증**
|
||||
|
||||
\```bash
|
||||
# 정적 분석
|
||||
<analyze command>
|
||||
# 테스트
|
||||
<test command>
|
||||
\```
|
||||
|
||||
Expected: <N개 통과>
|
||||
|
||||
---
|
||||
|
||||
## 의존 관계 및 구현 순서 ← 순서가 있을 때만 포함
|
||||
|
||||
\```
|
||||
[TAG-1] → [TAG-2] → [TAG-3]
|
||||
\```
|
||||
|
||||
<각 의존성이 존재하는 이유>
|
||||
|
||||
---
|
||||
|
||||
## 수정 파일 요약
|
||||
|
||||
| 파일 | 항목 |
|
||||
|------|------|
|
||||
| `path/to/file` | TAG-1, TAG-2 |
|
||||
|
||||
---
|
||||
|
||||
## 최종 검증
|
||||
|
||||
\```bash
|
||||
# 1. 정적 분석
|
||||
<analyze command>
|
||||
# 2. 전체 테스트
|
||||
<test command>
|
||||
\```
|
||||
|
||||
Expected outcome: <기존 N개 + 신규 M개 = 총 K개 통과>
|
||||
```
|
||||
|
||||
**Plan quality rules:**
|
||||
- Exact line numbers in every "Before" snippet.
|
||||
- `@override` wherever an abstract method is implemented.
|
||||
- Full import statement when a new package is added.
|
||||
- Every renamed symbol: list all call sites that must change.
|
||||
- "Add tests as needed" is forbidden — decide and specify upfront.
|
||||
- File paths not actually read: verify with Read or Grep first.
|
||||
|
||||
---
|
||||
|
||||
## Step 5 — Write CODE_REVIEW.md Stub
|
||||
|
||||
Write `tasks/{task_name}/CODE_REVIEW.md`:
|
||||
|
||||
```markdown
|
||||
<!-- task={task_name} plan=N tag=<TAG> -->
|
||||
|
||||
# Code Review Reference — <TAG>
|
||||
|
||||
## 개요
|
||||
|
||||
task={task_name}, plan=N, tag=TAG
|
||||
|
||||
## 구현 항목별 완료 여부
|
||||
|
||||
| 항목 | 완료 | 비고 |
|
||||
|------|------|------|
|
||||
| [TAG-1] <title> | [ ] | |
|
||||
| [TAG-2] <title> | [ ] | |
|
||||
|
||||
## 계획 대비 변경 사항
|
||||
|
||||
<벗어난 내용과 이유. 그대로면 "계획서 그대로 구현" 기재.>
|
||||
|
||||
## 주요 설계 결정
|
||||
|
||||
<구현 중 발생한 결정 사항.>
|
||||
|
||||
## 리뷰어를 위한 체크포인트
|
||||
|
||||
<집중해야 할 파일·줄 번호·로직.>
|
||||
|
||||
## 검증 결과
|
||||
|
||||
\```
|
||||
<실제 실행한 명령과 출력>
|
||||
\```
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Naming Convention
|
||||
|
||||
| Tag | Use for |
|
||||
|-----|---------|
|
||||
| `DART_API` | Dart public API changes |
|
||||
| `GO_REFACTOR` | Go internal refactoring |
|
||||
| `TEST` | Test additions or fixes |
|
||||
| `REVIEW_<TAG>` | Follow-up fixes after a code review |
|
||||
|
||||
---
|
||||
|
||||
## Final Verification Checklist
|
||||
|
||||
After writing both files:
|
||||
|
||||
1. `tasks/{task_name}/PLAN.md` exists and line 1 is `<!-- task={task_name} plan=N tag=TAG -->`.
|
||||
2. `tasks/{task_name}/CODE_REVIEW.md` exists and line 1 matches `<!-- task={task_name} plan=N tag=TAG -->`.
|
||||
3. Previous `PLAN.md` (if existed) was archived as `plan_N.log` with N = count of pre-existing logs.
|
||||
4. Previous `CODE_REVIEW.md` (if existed) was archived as `code_review_M.log`.
|
||||
5. Every plan item has all required sub-sections: 문제, 해결 방법, 수정 파일 체크리스트, 테스트 작성, 중간 검증.
|
||||
6. Every "테스트 작성" section has an explicit decision (write / skip) with justification.
|
||||
7. Completion table in `CODE_REVIEW.md` lists every plan item.
|
||||
Loading…
Reference in a new issue