diff --git a/skills/add-toki-socket-crosstest-language/SKILL.md b/skills/add-toki-socket-crosstest-language/SKILL.md index ed481c6..2a847ee 100644 --- a/skills/add-toki-socket-crosstest-language/SKILL.md +++ b/skills/add-toki-socket-crosstest-language/SKILL.md @@ -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 /crosstest/_. /crosstest/__client. ``` -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 =TestData` before scenarios. -- Print exactly one result line per scenario in this format: +- Connect to requested mode/port. +- Print `INFO typeName =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. diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md index 98910ba..390f05c 100644 --- a/skills/code-review/SKILL.md +++ b/skills/code-review/SKILL.md @@ -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_`. +- `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]** `:` — . 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: `` -- 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: `` (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. diff --git a/skills/plan/SKILL.md b/skills/plan/SKILL.md index 849c4c3..5d16bbe 100644 --- a/skills/plan/SKILL.md +++ b/skills/plan/SKILL.md @@ -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 - -# - -## 이 파일을 읽는 구현 에이전트에게 - -- 각 항목의 체크리스트를 순서대로 완료한다. -- 항목 완료마다 **중간 검증** 커맨드를 실행하고 통과를 확인한다. -- 모든 항목 완료 후 **최종 검증** 커맨드를 실행한다. -- 작업이 끝나면 `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.