diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md new file mode 100644 index 0000000..24cbed7 --- /dev/null +++ b/skills/code-review/SKILL.md @@ -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]** `:` — . 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: `` +- 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: `` (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. diff --git a/skills/implement/SKILL.md b/skills/implement/SKILL.md new file mode 100644 index 0000000..96f342f --- /dev/null +++ b/skills/implement/SKILL.md @@ -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 +# + +<!-- 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.