Update skill documentation
This commit is contained in:
toki
parent
1db698ad61
commit
34d06258c6
3 changed files with 226 additions and 447 deletions
|
|
@ -5,22 +5,20 @@ description: Create or extend toki_socket cross-language integration tests for a
|
|||
|
||||
# Add Toki Socket Crosstest Language
|
||||
|
||||
## Overview
|
||||
## Purpose
|
||||
|
||||
Use this skill to add cross-language compatibility tests for a new `toki_socket` language implementation. Keep crosstest code inside each language package, preserve the root for shared documentation, and match the Dart/Go baseline behavior.
|
||||
Add Toki Socket cross-language compatibility tests for a new language implementation. Keep crosstest code inside each language package, preserve the repository root for shared docs, and match the Dart/Go baseline behavior.
|
||||
|
||||
## Folder Rules
|
||||
|
||||
Place each language-specific runner or helper under that language's `crosstest` folder.
|
||||
|
||||
Use this naming pattern:
|
||||
Use package-local `crosstest` folders:
|
||||
|
||||
```text
|
||||
<server-language>/crosstest/<server>_<client>.<ext>
|
||||
<client-language>/crosstest/<server>_<client>_client.<ext-or-main-dir>
|
||||
```
|
||||
|
||||
Examples from the baseline:
|
||||
Baseline examples:
|
||||
|
||||
```text
|
||||
dart/crosstest/dart_go.dart
|
||||
|
|
@ -29,27 +27,25 @@ go/crosstest/go_dart.go
|
|||
dart/crosstest/go_dart_client.dart
|
||||
```
|
||||
|
||||
Avoid root-level runners such as `dart-go.dart` or `go-dart.go` unless the user explicitly asks for them. Do not add root-level package metadata (`pubspec.yaml`, `go.mod`, etc.) only to run crosstests. Run crosstests from the owning language package instead.
|
||||
|
||||
Keep only shared review notes or planning docs at the repository root. When changing paths from a plan, record why in the review note.
|
||||
Avoid root-level runners and root-level package metadata unless explicitly requested. If a plan path changes, record why in the review note.
|
||||
|
||||
## Test Shape
|
||||
|
||||
Create one orchestrator in the server language and one subprocess client helper in the client language.
|
||||
Create a server-language orchestrator plus a client-language subprocess helper.
|
||||
|
||||
The orchestrator must:
|
||||
|
||||
- Start a TCP server and run the client helper as a subprocess.
|
||||
- Start a WebSocket server and run the client helper as a subprocess.
|
||||
- Parse subprocess stdout for `PASS` and `FAIL` lines.
|
||||
- Fail if any expected scenario is missing, any `FAIL` line appears, or the subprocess exits non-zero.
|
||||
- Stop servers even when a subprocess fails.
|
||||
- Start TCP and WebSocket servers.
|
||||
- Run the client helper for each protocol/phase.
|
||||
- Parse subprocess stdout for `PASS` and `FAIL`.
|
||||
- Fail on missing expected scenario, any `FAIL`, or non-zero subprocess exit.
|
||||
- Stop servers even on subprocess failure.
|
||||
|
||||
The subprocess client must:
|
||||
The client helper must:
|
||||
|
||||
- Connect to the requested mode and port.
|
||||
- Print `INFO typeName <language>=TestData` before scenarios.
|
||||
- Print exactly one result line per scenario in this format:
|
||||
- Connect to requested mode/port.
|
||||
- Print `INFO typeName <language>=TestData`.
|
||||
- Print exactly one result line per scenario:
|
||||
|
||||
```text
|
||||
PASS scenario=N detail=...
|
||||
|
|
@ -58,42 +54,42 @@ FAIL scenario=N error=...
|
|||
|
||||
## Required Scenarios
|
||||
|
||||
Run these scenarios for both TCP and WebSocket:
|
||||
Run for both TCP and WebSocket:
|
||||
|
||||
| Scenario | Requirement |
|
||||
| --- | --- |
|
||||
|----------|-------------|
|
||||
| 1 | Client sends fire-and-forget `TestData`; server validates `index` and `message`. |
|
||||
| 2 | Server pushes `TestData(index=200, message=push from ... server)` to the client. |
|
||||
| 3 | Client `sendRequest` receives `index=req.index*2` and `message=echo: req.message`. |
|
||||
| 4 | Multiple concurrent `sendRequest` calls verify nonce/responseNonce routing. |
|
||||
|
||||
Split each protocol into separate `send-push` and `requests` phases when the language implementation forbids registering a normal listener and request listener for the same protobuf type at the same time. This is required for the current Dart and Go communicators.
|
||||
Split each protocol into `send-push` and `requests` phases when normal listeners and request listeners cannot coexist for one protobuf type. This is required for current Dart and Go communicators.
|
||||
|
||||
## Secure Transport Scenarios
|
||||
|
||||
When the new language implementation supports secure transports, add the same coverage for TLS+TCP and WSS, or document why the runtime cannot support them yet.
|
||||
When the language supports secure transports, add TLS+TCP and WSS coverage, or document why they are deferred.
|
||||
|
||||
At minimum, secure scenarios must verify:
|
||||
Secure scenarios must verify:
|
||||
|
||||
- A TLS+TCP client can connect to a TLS+TCP server and exchange `TestData`.
|
||||
- A WSS client can connect to a WSS server and exchange `TestData`.
|
||||
- Certificate/test trust setup stays inside test fixtures and does not depend on machine-global trust stores.
|
||||
- Secure test commands are included in the review note with any environment-specific caveats.
|
||||
- TLS+TCP client/server exchange `TestData`.
|
||||
- WSS client/server exchange `TestData`.
|
||||
- Certificates/trust setup stays in test fixtures, not machine-global stores.
|
||||
- Review notes include secure test commands and environment caveats.
|
||||
|
||||
If secure transport support is intentionally deferred, record it as a review note and make sure the language implementation README does not claim TLS/WSS availability.
|
||||
If deferred, ensure the language README does not claim TLS/WSS availability.
|
||||
|
||||
## Protocol Details
|
||||
|
||||
Use the generated `TestData` protobuf from each language package. The current proto has no `package` declaration, so both sides must use the simple type name:
|
||||
Use generated `TestData`. Current proto has no `package`; both sides must use simple type names:
|
||||
|
||||
```text
|
||||
Dart: TestData.getDefault().info_.qualifiedMessageName -> TestData
|
||||
Go: toki.TypeNameOf(&packets.TestData{}) -> TestData
|
||||
```
|
||||
|
||||
Log the type name from both sides during each crosstest run.
|
||||
Log type names from both sides.
|
||||
|
||||
Use fixed non-conflicting ports per language pair and protocol. Prefer a new documented range instead of reusing unit-test ports. If extending the Dart/Go baseline, keep:
|
||||
Use non-conflicting fixed ports. Keep the Dart/Go baseline ports when extending it:
|
||||
|
||||
```text
|
||||
Dart server / Go client TCP: 29090
|
||||
|
|
@ -104,33 +100,33 @@ Go server / Dart client WS: 29192
|
|||
|
||||
## Path Robustness
|
||||
|
||||
Resolve subprocess paths from the orchestrator source location or from the language package root. Do not assume the user starts the command from the repository root.
|
||||
Resolve subprocess paths from orchestrator source location or language package root. Do not assume the command starts at repository root.
|
||||
|
||||
Prefer commands like:
|
||||
Prefer package-local commands:
|
||||
|
||||
```bash
|
||||
(cd dart && dart run crosstest/dart_go.dart)
|
||||
(cd go && go run ./crosstest)
|
||||
```
|
||||
|
||||
For additional languages, provide the equivalent package-local command in the review note.
|
||||
For new languages, include equivalent commands in the review note.
|
||||
|
||||
## Verification
|
||||
|
||||
After implementation, run:
|
||||
Run:
|
||||
|
||||
- The new cross-language command in both directions.
|
||||
- The language package's existing unit tests.
|
||||
- The language package's formatter/analyzer/compiler checks.
|
||||
- New cross-language command in both directions.
|
||||
- Existing unit tests for each touched language package.
|
||||
- Formatter/analyzer/compiler checks for each touched language package.
|
||||
|
||||
If the local environment needs special PATH, cache, or sandbox settings, report that separately in the final answer and review note without baking machine-specific paths into the source.
|
||||
Report PATH/cache/sandbox caveats in final answer and review note; do not bake machine-specific paths into source.
|
||||
|
||||
## Review Note
|
||||
|
||||
Leave or update a root-level review note when the task requests reviewer guidance. Include:
|
||||
When reviewer guidance is requested, include:
|
||||
|
||||
- Added files and their roles.
|
||||
- Path decision and any deviation from an earlier plan.
|
||||
- Scenario coverage.
|
||||
- Exact package-local commands used for validation.
|
||||
- Any environment-specific caveats.
|
||||
- Added files and roles.
|
||||
- Path decisions and deviations.
|
||||
- Scenario coverage, including secure transports or deferral reason.
|
||||
- Exact package-local validation commands.
|
||||
- Environment-specific caveats.
|
||||
|
|
|
|||
|
|
@ -5,236 +5,134 @@ description: Review completed implementation work in the current repository. Rea
|
|||
|
||||
# Code Review
|
||||
|
||||
## Overview
|
||||
## Purpose
|
||||
|
||||
This skill is the third step in the plan → code → review loop:
|
||||
Review the implementation phase of the plan-code-review loop:
|
||||
|
||||
```text
|
||||
plan skill -> implementation -> code-review skill
|
||||
^ |
|
||||
+----- issues found: new PLAN.md
|
||||
```
|
||||
plan 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.
|
||||
|
||||
## Workflow Contract
|
||||
|
||||
This skill expects active work to live at `tasks/{task_name}/PLAN.md` and `tasks/{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.
|
||||
Active work must live at `tasks/{task_name}/PLAN.md` and `tasks/{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:
|
||||
|
||||
## Step 1 — Find the Active Task
|
||||
| State | Meaning |
|
||||
|-------|---------|
|
||||
| `PLAN.md` + `CODE_REVIEW.md` | Ready for review |
|
||||
| Only `*.log` files | Task complete |
|
||||
|
||||
1. Glob `tasks/*/CODE_REVIEW.md`.
|
||||
2. If exactly one exists: that is the active task. Record `task_name`. `PLAN.md` is always present alongside `CODE_REVIEW.md` — this is an invariant guaranteed by the plan skill and code-review skill. No existence check needed.
|
||||
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.
|
||||
The implementing agent never archives or deletes active files; archiving is this skill's responsibility.
|
||||
|
||||
---
|
||||
## Step 1 - Find Active Task
|
||||
|
||||
## Step 2 — Load Context
|
||||
Glob `tasks/*/CODE_REVIEW.md`:
|
||||
|
||||
The reading strategy depends on whether this is the first review or a follow-up.
|
||||
| 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 |
|
||||
|
||||
### 2-1. Determine review round
|
||||
## Step 2 - Load Context
|
||||
|
||||
Count `tasks/{task_name}/code_review_*.log` files.
|
||||
- Count = 0 → **First review**: no prior diff to reference. Read the full codebase as described in 2-2A.
|
||||
- Count ≥ 1 → **Follow-up review**: use git diff to anchor the search, then expand as described in 2-2B.
|
||||
Count `tasks/{task_name}/code_review_*.log`:
|
||||
|
||||
### 2-2A. First review — full analysis
|
||||
- `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.
|
||||
|
||||
No git narrowing. Read everything relevant to the plan:
|
||||
The diff is the starting point, not the boundary. Follow behavior and API connections far enough to judge correctness.
|
||||
|
||||
1. `tasks/{task_name}/CODE_REVIEW.md`
|
||||
2. `tasks/{task_name}/PLAN.md`
|
||||
3. Every source file listed in the plan's "수정 파일 및 체크리스트" — read the whole file.
|
||||
4. Every test file that exercises the changed code.
|
||||
5. Files that import or are imported by the changed files — **up to 2 levels deep**.
|
||||
## Step 3 - Pre-Review Checklist
|
||||
|
||||
### 2-2B. Follow-up review — diff-anchored + expansion
|
||||
Before writing the verdict:
|
||||
|
||||
Start with git unstaged changes to identify what changed, then expand outward to connected code.
|
||||
- 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.
|
||||
|
||||
The implementing agent does not commit — changes are always in the working tree (unstaged or staged).
|
||||
## Step 4 - Append Verdict
|
||||
|
||||
**Collect the diff:**
|
||||
```bash
|
||||
git diff # unstaged changes
|
||||
git diff --cached # staged changes
|
||||
git log --oneline -5
|
||||
```
|
||||
Append `코드리뷰 결과` to `CODE_REVIEW.md`.
|
||||
|
||||
Use the union of both as the change set.
|
||||
Required fields:
|
||||
|
||||
**Expand beyond the diff** — for each changed file or symbol, also read:
|
||||
- Files that **import** the changed file — **up to 2 levels deep** (callers may be broken by signature changes)
|
||||
- Files that **implement or extend** a changed interface or abstract class
|
||||
- Test files that **exercise** the changed logic, even if not modified themselves
|
||||
- Any file the plan's "수정 파일 및 체크리스트" lists that does not appear in the diff (planned but possibly missing)
|
||||
- `종합 판정`: 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.
|
||||
|
||||
The diff scopes where to start; it does not limit where to stop. Follow the connections.
|
||||
Severity semantics:
|
||||
|
||||
> Note: import expansion boundary follows the project's structural rules (e.g., DDD layer rules). If a separate rules file defines allowed dependency directions, use it as the ceiling for how far to trace.
|
||||
| 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:
|
||||
|
||||
## Step 3 — Pre-Review Checklist
|
||||
- `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.
|
||||
|
||||
Complete every item before writing the verdict.
|
||||
## Step 5 - Archive Active Files
|
||||
|
||||
**Applies to all reviews:**
|
||||
- [ ] Compare actual source files against the plan's "수정 파일 및 체크리스트" — are all planned changes present in the code?
|
||||
- [ ] For every symbol renamed or removed: Grep on the current working tree to confirm no stale references remain.
|
||||
- [ ] For every new test listed in the plan: confirm it exists, name matches, assertion is non-trivial.
|
||||
- [ ] Cross-check the claimed verification output in CODE_REVIEW.md against the actual code and project commands.
|
||||
Archive order is fixed:
|
||||
|
||||
**Follow-up review only** (count ≥ 1, diff available):
|
||||
- [ ] Compare `git diff` + `git diff --cached` against the plan's checklist — are there unplanned changes or missing changes?
|
||||
- [ ] Scan the diff for noise: debug prints, dead code, leftover TODOs, formatting-only changes, unrelated edits.
|
||||
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 4 — Append Verdict to CODE_REVIEW.md
|
||||
## Step 6 - Post-Review Actions
|
||||
|
||||
Append the following section to the existing `CODE_REVIEW.md`. Every field is required.
|
||||
For `PASS`, report:
|
||||
|
||||
```markdown
|
||||
---
|
||||
- Verdict.
|
||||
- Archive filenames.
|
||||
- Task complete; only `.log` files remain.
|
||||
|
||||
## 코드리뷰 결과
|
||||
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.
|
||||
- New `CODE_REVIEW.md` stub must use the same header and list every new plan item unchecked.
|
||||
|
||||
**[ PASS | WARN | FAIL ]**
|
||||
Report Required/Suggested counts, archive names, and the new plan path.
|
||||
|
||||
> <1–2 sentences summarising the overall assessment.>
|
||||
## Review Dimensions
|
||||
|
||||
### 차원별 평가
|
||||
|
||||
| 차원 | 판정 | 비고 |
|
||||
|------|------|------|
|
||||
| 정확성 (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]** Required/Suggested 없음. Nit만 있으면 별도 계획 없이 아카이브 완료.
|
||||
**[WARN]** Suggested N건. 새 PLAN.md 작성 완료. 구현 에이전트는 PLAN.md를 읽고 작업 시작.
|
||||
**[FAIL]** 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/Suggested)
|
||||
|
||||
No further files needed. Nit-only findings may be listed in the verdict and still PASS. 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
|
||||
|
||||
WARN = one or more Suggested issues, zero Required. FAIL = one or more Required issues. Nit-only findings do not force WARN and may PASS. WARN and FAIL unconditionally write new PLAN.md and CODE_REVIEW.md stub.
|
||||
|
||||
Write new files for the fix round:
|
||||
|
||||
**Write `tasks/{task_name}/PLAN.md`** following the plan 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.
|
||||
- If verdict is WARN (Suggested issues, no Required): group all Suggested issues, plus any related Nit issues, into one plan item. Include it in the plan — it is not optional.
|
||||
- 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: FAIL → Required N건 / WARN → Suggested N건
|
||||
- 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 |
|
||||
|
||||
---
|
||||
| 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
|
||||
|
||||
**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.
|
||||
- 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.
|
||||
|
||||
**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 Checklist
|
||||
|
||||
---
|
||||
|
||||
## 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 WARN or FAIL: new `PLAN.md` exists, header `plan=N` matches count of `plan_*.log` after archiving. FAIL: every Required issue has its own plan item with full sub-sections. WARN: grouped Suggested plan item exists with full sub-sections.
|
||||
6. If WARN or FAIL: new `CODE_REVIEW.md` stub exists, header `plan=N` matches new PLAN.md, completion table lists every item.
|
||||
- `code_review_N.log` exists with verdict appended.
|
||||
- `plan_M.log` exists.
|
||||
- No active `.md` files remain after PASS.
|
||||
- WARN/FAIL created new active `PLAN.md` and `CODE_REVIEW.md` with matching headers.
|
||||
|
|
|
|||
|
|
@ -1,262 +1,147 @@
|
|||
---
|
||||
name: plan
|
||||
description: Analyze the current repository and write a detailed PLAN.md for implementation work. Also writes the CODE_REVIEW.md stub that the implementing agent will fill in after coding. Use for any feature, refactor, bug fix, or follow-up fix that should enter the plan-code-review loop. A separate agent reads PLAN.md and does the actual coding. The code-review skill archives both files after review.
|
||||
description: Analyze the current repository and write a detailed PLAN.md for implementation work. Also writes the CODE_REVIEW.md stub that the implementing agent will fill in after coding. Use for any feature, refactor, bug fix, or follow-up fix that should enter the plan-code-review loop. A separate implementing agent, or the same agent in an implementation pass, reads PLAN.md and does the coding. The code-review skill archives both files after review.
|
||||
---
|
||||
|
||||
# Plan
|
||||
|
||||
## Overview
|
||||
## Purpose
|
||||
|
||||
This skill produces two files that drive the implementation loop:
|
||||
Create the planning artifacts for the implementation loop:
|
||||
|
||||
```text
|
||||
plan skill -> PLAN.md + CODE_REVIEW.md stub
|
||||
implementation -> code changes + filled CODE_REVIEW.md
|
||||
code-review skill -> verdict + archive, or new follow-up PLAN.md
|
||||
```
|
||||
plan 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 — plan 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 |
|
||||
|
||||
## Workflow Contract
|
||||
|
||||
This skill intentionally uses `tasks/{task_name}/PLAN.md` and `tasks/{task_name}/CODE_REVIEW.md` as the state protocol between planning, implementation, and review. Do not change these paths or filenames unless the paired plan and code-review skills are updated together. This convention is not project-specific; it is the workflow contract for this skill loop.
|
||||
|
||||
---
|
||||
Directory states:
|
||||
|
||||
## Step 1 — Determine Task
|
||||
| State | Meaning |
|
||||
|-------|---------|
|
||||
| `PLAN.md` only | Invalid; plan skill always writes both files |
|
||||
| `PLAN.md` + `CODE_REVIEW.md` stub | Implementation is pending/in progress |
|
||||
| `PLAN.md` + filled `CODE_REVIEW.md` | Ready for code-review skill |
|
||||
| Only `*.log` files | Task complete |
|
||||
|
||||
If the user names the task explicitly, use that name and skip auto-detection.
|
||||
## Step 1 - Determine Task
|
||||
|
||||
Otherwise, glob `tasks/*/PLAN.md` and branch:
|
||||
If the user names the task explicitly, use that task name.
|
||||
|
||||
Otherwise, glob `tasks/*/PLAN.md`:
|
||||
|
||||
| Result | Action |
|
||||
|--------|--------|
|
||||
| Exactly one found | Continuing task — use that directory. `task_name` = directory name. |
|
||||
| None found | New task — derive `task_name` from the user request (short snake_case, e.g. `api_refactor`). Create `tasks/{task_name}/`. |
|
||||
| Multiple found | Cannot auto-detect — list the found paths and ask the user which task to work on. Stop. |
|
||||
| Exactly one | Continue that task |
|
||||
| None | Create a new task only for a feature/refactor/fix/follow-up that belongs in this workflow |
|
||||
| Multiple | List paths and ask which task to use |
|
||||
|
||||
> PLAN.md is the entry point of the loop. Without it the loop has not started.
|
||||
> No `PLAN.md` should normally occur only before the first plan for a new task. Do not create a plan just because none exists; create one only when the user is starting a new feature/refactor/fix/follow-up that belongs in this workflow. For casual analysis, status, or review requests, answer without creating task files unless the user explicitly asks for a plan.
|
||||
`PLAN.md` is the loop entry point. A missing `PLAN.md` normally means only that no plan has been started for a new task; do not create task files for casual analysis, status, or review requests unless the user explicitly asks for a plan.
|
||||
|
||||
---
|
||||
Use short snake_case task names, e.g. `api_refactor`.
|
||||
|
||||
## Step 2 — Analyze Codebase
|
||||
## Step 2 - Analyze Before Writing
|
||||
|
||||
Complete every item before writing a single line. Do not skip.
|
||||
Complete these before creating files:
|
||||
|
||||
- [ ] 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 the repository's package manifests and dependency files before adding new dependencies.
|
||||
- [ ] Anticipate compile errors: missing `@override`, type mismatches, broken imports.
|
||||
- [ ] Confirm the final verification command works in this project layout.
|
||||
- Read every source file the change will touch, whole file.
|
||||
- Read every test file that exercises changed behavior.
|
||||
- For each behavior change, note whether existing tests cover it.
|
||||
- Grep renamed/removed symbols for all call sites and import chains.
|
||||
- Check package manifests/dependency files before adding dependencies.
|
||||
- Anticipate compile issues: missing overrides, type mismatches, broken imports.
|
||||
- Confirm final verification commands work in this repository layout.
|
||||
|
||||
---
|
||||
## Step 3 - Archive Existing Active Files
|
||||
|
||||
## Step 3 — Archive Previous Files (if any)
|
||||
Before writing new active files for the chosen task:
|
||||
|
||||
Before writing new files, archive any active files from a previous round:
|
||||
- Count existing `tasks/{task_name}/plan_*.log`; call it `N`. If `PLAN.md` exists, rename it to `plan_N.log`.
|
||||
- Count existing `tasks/{task_name}/code_review_*.log`; call it `M`. If `CODE_REVIEW.md` exists, rename it to `code_review_M.log`.
|
||||
|
||||
**Archive PLAN.md:**
|
||||
- Count existing `tasks/{task_name}/plan_*.log` → that count is N.
|
||||
- If `PLAN.md` exists, rename it to `plan_N.log`.
|
||||
The new plan number is the count of `plan_*.log` after archiving.
|
||||
|
||||
**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
|
||||
|
||||
---
|
||||
|
||||
## Step 4 — Write PLAN.md
|
||||
|
||||
**N for the header** = count of `tasks/{task_name}/plan_*.log` files after Step 3 (i.e., including the one just archived, if any).
|
||||
|
||||
Write `tasks/{task_name}/PLAN.md`:
|
||||
|
||||
> Note: Include the "의존 관계 및 구현 순서" section only when plan items must be done in a specific order. Omit it entirely when items are independent.
|
||||
Header line must be exactly:
|
||||
|
||||
```markdown
|
||||
<!-- task={task_name} plan=N tag=<TAG> -->
|
||||
# <Title>
|
||||
|
||||
## 이 파일을 읽는 구현 에이전트에게
|
||||
|
||||
- 각 항목의 체크리스트를 순서대로 완료한다.
|
||||
- 항목 완료마다 **중간 검증** 커맨드를 실행하고 통과를 확인한다.
|
||||
- 모든 항목 완료 후 **최종 검증** 커맨드를 실행한다.
|
||||
- 작업이 끝나면 `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개 통과>
|
||||
<!-- task={task_name} plan=N tag=TAG -->
|
||||
```
|
||||
|
||||
**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.
|
||||
Required sections:
|
||||
|
||||
---
|
||||
- Title.
|
||||
- `이 파일을 읽는 구현 에이전트에게`: tell the implementer to complete checklists, run intermediate/final verification, and fill every `CODE_REVIEW.md` section with actual implementation notes and command output.
|
||||
- `배경`: 2-4 sentences explaining why the work is needed.
|
||||
- One item per change: `### [TAG-1] Title`, `TAG-2`, etc.
|
||||
- `수정 파일 요약`: table mapping files to item ids.
|
||||
- `최종 검증`: runnable commands and expected outcome.
|
||||
|
||||
## Step 5 — Write CODE_REVIEW.md Stub
|
||||
Each plan item must include:
|
||||
|
||||
Write `tasks/{task_name}/CODE_REVIEW.md`:
|
||||
- `문제`: concrete problem with file:line references.
|
||||
- `해결 방법`: exact approach and before/after code block for non-trivial changes.
|
||||
- `수정 파일 및 체크리스트`: exhaustive file-level checklist.
|
||||
- `테스트 작성`: explicit write/skip decision. If writing tests, include path, test name, assertion goal, and fixtures. If skipping, justify.
|
||||
- `중간 검증`: runnable commands and expected result.
|
||||
|
||||
Include `의존 관계 및 구현 순서` only when order matters.
|
||||
|
||||
Quality rules:
|
||||
|
||||
- Exact line numbers in every Before snippet.
|
||||
- Use the language's required override annotation/keyword wherever an abstract method is implemented.
|
||||
- Include full import statements for new packages.
|
||||
- List all call sites for renamed/removed symbols.
|
||||
- Never write "add tests as needed"; decide up front.
|
||||
- Do not cite files you did not read.
|
||||
|
||||
Test policy:
|
||||
|
||||
| Change | Test requirement |
|
||||
|--------|------------------|
|
||||
| Bug fix | Regression test required |
|
||||
| New public API | Normal + boundary tests required |
|
||||
| API rename | Existing test call-site updates usually enough |
|
||||
| Internal refactor | Existing tests may be enough |
|
||||
| Concurrency logic | Race/ordering test recommended |
|
||||
|
||||
## Step 5 - Write CODE_REVIEW.md Stub
|
||||
|
||||
Header line must match PLAN.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> | [ ] | |
|
||||
|
||||
## 계획 대비 변경 사항
|
||||
|
||||
<벗어난 내용과 이유. 그대로면 "계획서 그대로 구현" 기재.>
|
||||
|
||||
## 주요 설계 결정
|
||||
|
||||
<구현 중 발생한 결정 사항.>
|
||||
|
||||
## 리뷰어를 위한 체크포인트
|
||||
|
||||
<집중해야 할 파일·줄 번호·로직.>
|
||||
|
||||
## 검증 결과
|
||||
|
||||
\```
|
||||
<실제 실행한 명령과 출력>
|
||||
\```
|
||||
<!-- task={task_name} plan=N tag=TAG -->
|
||||
```
|
||||
|
||||
---
|
||||
Required sections:
|
||||
|
||||
## Naming Convention
|
||||
- `# Code Review Reference - TAG`
|
||||
- `개요`: `task={task_name}, plan=N, tag=TAG`
|
||||
- `구현 항목별 완료 여부`: one row per plan item, unchecked.
|
||||
- `계획 대비 변경 사항`
|
||||
- `주요 설계 결정`
|
||||
- `리뷰어를 위한 체크포인트`
|
||||
- `검증 결과`
|
||||
|
||||
## Naming
|
||||
|
||||
| Tag | Use for |
|
||||
|-----|---------|
|
||||
| `API` | Public API changes |
|
||||
| `REFACTOR` | Internal refactoring |
|
||||
| `TEST` | Test additions or fixes |
|
||||
| `REVIEW_<TAG>` | Follow-up fixes after a code review |
|
||||
| `TEST` | Test additions/fixes |
|
||||
| `REVIEW_<TAG>` | Follow-up fixes after review |
|
||||
|
||||
---
|
||||
## Final Checklist
|
||||
|
||||
## 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.
|
||||
- `PLAN.md` and `CODE_REVIEW.md` both exist under `tasks/{task_name}/`.
|
||||
- Both first lines match `<!-- task={task_name} plan=N tag=TAG -->`.
|
||||
- Previous active files, if any, were archived with correct numeric suffixes.
|
||||
- Every plan item has problem, solution, checklist, test decision, and intermediate verification.
|
||||
- `CODE_REVIEW.md` completion table lists every plan item.
|
||||
|
|
|
|||
Loading…
Reference in a new issue