343 lines
36 KiB
Markdown
343 lines
36 KiB
Markdown
---
|
|
name: code-review
|
|
description: "Use for active task review requests such as 리뷰 진행해, 리뷰해줘, 코드 리뷰해줘, code review, CODE_REVIEW.md, USER_REVIEW.md resolution, implementation-time 사용자 리뷰 요청, or 리뷰 루프. Review and archive one active pair, then leave exactly one next artifact: a follow-up PLAN/CODE_REVIEW pair, USER_REVIEW.md, or complete.log. Plan, implementation, and review use different models."
|
|
---
|
|
|
|
# Code Review
|
|
|
|
## Purpose
|
|
|
|
Review the implementation phase of the plan-code-review loop:
|
|
|
|
```text
|
|
plan model -> implementation model -> review model
|
|
^ |
|
|
+-- different plan model on WARN/FAIL -+
|
|
```
|
|
|
|
Implementation may end its coding phase early by filling the active review stub's `사용자 리뷰 요청` section from `agent-ops/skills/common/_templates/implementation-user-review-request-section.md`. This path is valid only for a selected Milestone `구현 잠금 > 결정 필요` item. The implementation model must not ask the user directly, call a user-input tool, or execute code-review. It fills every implementation-owned section, reports `review-ready`, and stops for a later review model.
|
|
|
|
The review agent must select exactly one next-state artifact before ending:
|
|
|
|
- **Plan**: a different plan model writes one follow-up PLAN/CODE_REVIEW pair.
|
|
- **User review**: the review agent writes `USER_REVIEW.md`.
|
|
- **Complete**: the review agent writes `complete.log` and archives the task.
|
|
|
|
Archived findings or `plan-ready` alone are not a valid review result.
|
|
The three-output rule begins only after a `review-ready` pair or resolvable `USER_REVIEW.md` passes preflight. An unfilled stub remains in the implementation stage and is not a started review.
|
|
Finalization recovery uses the durable artifact combination itself: verdict-appended active review, partial pair archive, `complete.log`, `USER_REVIEW.md`, resolved `user_review_*.log`, or archived WARN/FAIL logs. No model-written transient state file is part of the protocol.
|
|
|
|
## Hard Actor Boundary
|
|
|
|
- Plan, implementation, and review use different model identities; separate agents or reasoning settings on the same model do not satisfy this rule.
|
|
- Runtime must preserve the exact selected task path and runtime-provided model identities through every handoff. A filled role identity is immutable for that pair; a retry uses the same model identity. Lane/G and agent names are not model identities.
|
|
- Resolving `USER_REVIEW.md` continues the archived Review role and must use the same Review model identity recorded in that archived review. If that identity is unavailable, leave `USER_REVIEW.md` unchanged and stop.
|
|
- The review model owns the verdict, archive, and USER_REVIEW/complete finalization. It must not edit product source/tests or author a follow-up plan.
|
|
- On WARN/FAIL, dispatch a plan model different from both the implementation and review models. End only after the follow-up pair exists; do not implement it.
|
|
|
|
## Core Loop Rules
|
|
|
|
- Trigger: Korean or English active-task review requests, including `리뷰 진행해` and `리뷰해줘`, must use this skill when an active `CODE_REVIEW-*-G??.md` or `USER_REVIEW.md` exists under `agent-task/*/` or `agent-task/*/*/`, excluding `agent-task/archive/**`.
|
|
- Keep the selected `{task_name}` through review finalization.
|
|
- `PASS`: archive the pair, write `complete.log`, and move the task to monthly archive.
|
|
- `WARN`/`FAIL`: apply the user-review gate, then archive the pair. Write `USER_REVIEW.md` for a valid gate; otherwise dispatch a different plan model to write the follow-up pair.
|
|
- A filled `사용자 리뷰 요청` is valid only for a selected Milestone lock decision. Otherwise record the blocker as a WARN/FAIL finding for the next plan model.
|
|
- Do not replace `USER_REVIEW.md` with an inline user question. When the gate triggers, write the file-based stop state and include its path in the final report.
|
|
- Every unresolved `Required` forces `FAIL`; every `Suggested` forces `WARN`. Both require a follow-up pair unless the user-review gate writes `USER_REVIEW.md`.
|
|
- Recovery: finish missing review-owned finalization, then dispatch the different plan model when WARN/FAIL lacks a follow-up pair. Never plan or implement with the review model.
|
|
|
|
## User Review Gate
|
|
|
|
`USER_REVIEW.md` is a loop stop state only for selected Milestone lock decisions. The default WARN/FAIL branch creates a follow-up pair through a different plan model. The gate requires positive evidence that the blocker is already represented as a concrete selected Milestone `구현 잠금 > 결정 필요` item.
|
|
|
|
- Derive the pending review archive suffix as `0` when no `code_review_*.log` exists, otherwise `max(existing numeric suffix) + 1`; set `review-number` to that suffix plus one.
|
|
- Repeated `WARN`/`FAIL`, missing verification evidence, and environment blockers do not trigger `USER_REVIEW.md`; record them as findings for the next plan model.
|
|
- If the active review file has a filled `사용자 리뷰 요청` with `상태` other than `없음`, validate that `사유 유형` is `milestone-lock`, that `연결 대상` points to the selected Milestone, and that the requested decision matches a Milestone `구현 잠금 > 결정 필요` item.
|
|
- A valid Milestone-lock request is `Required`/`FAIL` and writes `USER_REVIEW.md` instead of a follow-up pair.
|
|
- If the request is not tied to the Milestone lock, do not write `USER_REVIEW.md`; record it as a WARN/FAIL finding.
|
|
- `USER_REVIEW.md` is created from `agent-ops/skills/common/code-review/templates/user-review-template.md`, filled with the archived loop history, current archived plan/review paths, verdict, loop count, blocking evidence, connection target, and the exact Milestone decision item.
|
|
|
|
## User Review Resolution
|
|
|
|
When an active `USER_REVIEW.md` exists, it remains terminal until the linked Milestone decision is resolved. After resolution, this skill either completes PASS finalization or dispatches a different plan model when more implementation is required.
|
|
|
|
- Read `USER_REVIEW.md`, archived `plan_*.log`, and archived `code_review_*.log` in that task directory.
|
|
- Inspect `## 상태` before writing. `USER_REVIEW` means the linked decision still needs first-time resolution. `RESOLVED` means resolution was already durably recorded; never append another result or replace its timestamp.
|
|
- When status is `USER_REVIEW`, verify the linked decision. If it remains unresolved, leave `USER_REVIEW.md` unchanged; otherwise record its resolution exactly once using the matching branch below.
|
|
- If a first-time resolution requires implementation, replace the `## 상태` value with `RESOLVED` and append one `## 해소 결과` with the exact keys `결과: REPLAN`, `해소 일시`, `결정`, `Evidence`, `Archived plan`, `Archived review`, `Forbidden implementation model`, and `Forbidden review model`; then continue immediately with the `RESOLVED`/`REPLAN` branch below.
|
|
- If active `USER_REVIEW.md` is already `RESOLVED`/`REPLAN`, validate that single result, archive the file unchanged as `user_review_N.log` using `N=0` or `max(existing numeric suffix) + 1`, then dispatch a different plan model with that log and its referenced archived pair. Verify the pair. The highest numeric-suffix matching `RESOLVED`/`REPLAN` log is durable `plan-ready` evidence and takes precedence over the original user-review gate during recovery.
|
|
- If a first-time resolution closes the task, select one completion timestamp, replace the `## 상태` value with `RESOLVED`, and append one `## 해소 결과` with the exact keys `결과: COMPLETE`, `해소 일시`, `Final verdict`, `결정`, `Evidence`, `Archived plan`, and `Archived review`; then continue immediately with the `RESOLVED`/`COMPLETE` branch below.
|
|
- If active `USER_REVIEW.md` is already `RESOLVED`/`COMPLETE`, validate that single result and reuse its `해소 일시` without changing the file. The final archive path is selected and persisted later in `complete.log`.
|
|
- On that complete/PASS branch, write `complete.log` from `agent-ops/skills/common/code-review/templates/complete-log-template.md` before moving or archiving the task directory. Include both the original archived review verdict and the user-review resolution line in `루프 이력`.
|
|
- Then apply the same task-directory archive move and `m-<milestone-slug>` PASS completion metadata rules as a normal `PASS`.
|
|
- Do not leave an active task directory that contains `USER_REVIEW.md` and `*.log` files but no `complete.log` after the linked Milestone decision resolves the task as complete/PASS.
|
|
|
|
## Workflow Contract
|
|
|
|
Active work must live under an active task directory using routed filenames. This is the state protocol shared with the plan skill.
|
|
|
|
Task path terms:
|
|
|
|
- `{task_group}` is the top-level work category under `agent-task/`. Normal task groups use a short snake_case name such as `refactoring`.
|
|
- Milestone-linked work uses the reserved task group form `m-<milestone-slug>`, where `<milestone-slug>` is the active Milestone filename without `.md`.
|
|
- `{subtask_dir}` is used only for split work and follows the indexed directory naming contract, such as `01_core` or `02+01_db`.
|
|
- `{subtask_name}` is the short snake_case name after the index or dependency prefix inside `{subtask_dir}`.
|
|
- `{task_name}` in headers and templates means the active task path relative to `agent-task/`: either `{task_group}` for a single-plan task or `{task_group}/{subtask_dir}` for a split subtask.
|
|
- A single-plan task stores active files directly under `agent-task/{task_group}/`.
|
|
- Split work stores active files under `agent-task/{task_group}/{subtask_dir}/`; the parent `agent-task/{task_group}/` is only the grouping folder and must not contain active plan/review files.
|
|
|
|
Filename rules:
|
|
|
|
- Plan file: `PLAN-{build_lane}-GNN.md`
|
|
- Review file: `CODE_REVIEW-{review_lane}-GNN.md`
|
|
- `{lane}` is only `local` or `cloud`; never put model names in filenames.
|
|
- `GNN` is a two-digit capability grade from `G01` to `G10`; runtime maps lane+grade to current models externally.
|
|
- This skill reads the active routed names but does not choose follow-up lane/G. Follow-up basenames come only from `plan` executing `finalize-task-routing`.
|
|
|
|
Multi-plan runtime contract:
|
|
|
|
- Multi-plan work is represented as multiple subtask directories under one shared `{task_group}`. Each subtask directory owns exactly one normal active plan file and one normal active review file.
|
|
- Multi-plan subtask directory names encode runtime scheduling metadata:
|
|
- `NN_{subtask_name}` has no runtime dependencies.
|
|
- `NN+PP[,QQ...]_{subtask_name}` depends on the listed earlier task indices.
|
|
- Subtask directory names are the runtime dependency source of truth. Preserve them verbatim; do not normalize, reinterpret, infer extra dependencies from numeric order, or choose execution order by agent judgment.
|
|
- If the user/runtime names a task group, task path, or subtask directory that identifies exactly one active review file, review that directory even when other active review files exist.
|
|
|
|
Milestone task group contract:
|
|
|
|
- `agent-task/m-<milestone-slug>/` is reserved for Milestone-linked work created by the plan skill.
|
|
- Do not treat normal task groups that do not start with `m-` as runtime milestone completion targets.
|
|
- For a selected task path, parse only the first path segment as `{task_group}`. If it matches `^m-[a-z0-9][a-z0-9-]*$`, strip `m-` to get `<milestone-slug>`.
|
|
- Do not modify `agent-roadmap/**` for milestone routing during code-review finalization. Read only the Milestone path from `Roadmap Targets` and its SDD path when needed to verify SDD Evidence Map.
|
|
- Do not call `update-roadmap` from this skill. The runtime consumes the PASS completion event, checks current state, resolves the active Milestone, and calls `update-roadmap` if needed.
|
|
- For `m-<milestone-slug>` PASS tasks, report the original active task path, final archive path, complete log path, task group, and milestone slug so the runtime has deterministic event inputs.
|
|
|
|
Follow-up handoff:
|
|
|
|
- Record concrete findings, affected paths, verification evidence, and release conditions in the archived review log.
|
|
- The review model must not choose the next lane/G or write the follow-up pair.
|
|
- Dispatch a different plan model with the exact archived log paths. That model performs isolated routing, writes the pair, reports `implementation-ready` or `dependency-waiting`, and stops.
|
|
- The review agent is responsible for ensuring that the follow-up pair exists, even though the different plan model authors it.
|
|
|
|
Directory states:
|
|
|
|
| State | Meaning |
|
|
|-------|---------|
|
|
| `PLAN-*-G??.md` + unfilled `CODE_REVIEW-*-G??.md` stub/placeholders | `implementation-ready`, `dependency-waiting`, or incomplete implementation; do not start review or append a verdict |
|
|
| `PLAN-*-G??.md` + `CODE_REVIEW-*-G??.md` with filled `사용자 리뷰 요청` | Implementation claims a selected Milestone lock decision blocker; review validates the request and writes `USER_REVIEW.md` only if justified |
|
|
| `PLAN-*-G??.md` + filled `CODE_REVIEW-*-G??.md` without verdict, dependencies satisfied | `review-ready`; the review model may consume it |
|
|
| Active pair with encoded predecessors still unsatisfied | `dependency-waiting`; leave the pair unchanged and do not start review |
|
|
| `PLAN-*-G??.md` + `CODE_REVIEW-*-G??.md` with appended verdict | Transitional; do not append another verdict and immediately resume Step 5 preparation/archive |
|
|
| Exactly one active pair member + its newly archived counterpart | Partial archive after a preflighted finalization; verify both identities, finish the remaining archive, then resume post-archive recovery |
|
|
| `complete.log` + `*.log` files | Task complete (PASS or user-review-resolved PASS), with final archive/event metadata fixed before the final task-directory move |
|
|
| `USER_REVIEW.md` + `*.log` files | Linked Milestone decision is required |
|
|
| Highest numeric-suffix matching `user_review_*.log` has `## 상태` value `RESOLVED` and `해소 결과 > 결과` value `REPLAN`, with no active pair | Resolved user-review `plan-ready`; dispatch a different plan model using that log and its referenced archived pair |
|
|
| `agent-task/archive/YYYY/MM/{task_name}/complete.log` + `*.log` files | Archived completed task path (PASS or user-review-resolved PASS); not active |
|
|
| Only matching `plan_*.log` / `code_review_*.log` with WARN/FAIL | Internal `plan-ready`; dispatch a different plan model to create the follow-up pair |
|
|
|
|
The implementing agent never archives or deletes active files; archiving is this skill's responsibility.
|
|
|
|
## Step 1 - Find Active Task
|
|
|
|
Find active review files with both globs, excluding `agent-task/archive/**`:
|
|
|
|
- `agent-task/*/CODE_REVIEW-*-G??.md`
|
|
- `agent-task/*/*/CODE_REVIEW-*-G??.md`
|
|
|
|
Also note active user-review stops, excluding `agent-task/archive/**`:
|
|
|
|
- `agent-task/*/USER_REVIEW.md`
|
|
- `agent-task/*/*/USER_REVIEW.md`
|
|
|
|
Classify the combined set of active `CODE_REVIEW-*-G??.md` and `USER_REVIEW.md` paths. Apply the first matching row:
|
|
|
|
| Result | Action |
|
|
|--------|--------|
|
|
| Exactly one active path and it is `CODE_REVIEW-*-G??.md` | Review that task; exactly one `PLAN-*-G??.md` is normally expected beside it. If the review already has a verdict and its exact plan counterpart was just archived, use partial-archive recovery instead of reporting a missing plan. |
|
|
| Exactly one active path and it is `USER_REVIEW.md`, with a linked Milestone completion/resolution decision | Perform User Review Resolution for that task. |
|
|
| One or more active paths and every active path is `USER_REVIEW.md`, with no completion/resolution decision | Report that the linked Milestone decision is required and list the paths. |
|
|
| No active paths | Apply the finalization-recovery scan below; stop only when it finds no recoverable task. |
|
|
| Multiple active paths | If the user/runtime named a task group, task path, or subtask directory that identifies exactly one active path, use that directory. Otherwise list paths and stop with an ambiguity report; do not choose by agent judgment and do not create a user-review request for routing ambiguity. |
|
|
|
|
If a selected task directory contains both `USER_REVIEW.md` and an active `PLAN-*-G??.md` or `CODE_REVIEW-*-G??.md`, do not overwrite either state or ask the user to choose between files. Validate provenance: a valid unresolved `USER_REVIEW.md` remains the terminal state; otherwise resume the provable active-pair or finalization recovery path and preserve the inconsistent path as review evidence.
|
|
|
|
Before review begins, runtime re-evaluates every encoded predecessor for both filled and unfilled pairs. If one is unsatisfied, leave the pair unchanged, report `dependency-waiting`, and do not record a Review identity or append a verdict.
|
|
|
|
An unfilled review stub with satisfied dependencies is not `review-ready`. Leave the pair unchanged and return it to the implementation stage.
|
|
|
|
Finalization-recovery scans active task paths and uses only the artifact identities already present:
|
|
|
|
- Find task directories with a verdict-appended active review, a partially archived pair, `complete.log` not yet moved, matching archived plan/review logs with no active pair, `USER_REVIEW.md`, or resolved `user_review_*.log`.
|
|
- Match plan/review artifacts by their `task` and `plan` headers, and match a resolved user-review log by the archived plan/review paths recorded in its `해소 결과`; do not choose by modification time alone.
|
|
- Recover the user/runtime-selected task path. Without one, recover only a single unambiguous candidate; list multiple candidates without mutating them.
|
|
- Apply recovery precedence in this order: active `complete.log`; active `USER_REVIEW.md`; an already-created follow-up pair; highest numeric-suffix matching `RESOLVED`/`REPLAN` user-review log; verdict-appended or partially archived pair; archived pair alone.
|
|
- Before returning an existing next artifact, verify its task/plan provenance and required fields.
|
|
- An active `complete.log` owns its completion timestamp and exact final archive path. Validate those fields, then resume the final move.
|
|
- An active `RESOLVED` `USER_REVIEW.md` resumes from its single `해소 결과`: archive an unchanged `REPLAN` result or continue `COMPLETE` using its fixed timestamp. Never resolve it again.
|
|
- A highest numeric-suffix matching `RESOLVED`/`REPLAN` user-review log dispatches the different plan model with that log and its referenced archived pair; never recreate `USER_REVIEW.md` from the older gate.
|
|
- Resume a verdict or partial archive at Step 5. For an archived WARN/FAIL pair without a next artifact, write `USER_REVIEW.md` when it proves a valid unresolved gate; otherwise dispatch the different plan model. For an archived PASS pair, finish the missing `complete.log`.
|
|
- A newly created unfilled follow-up pair already satisfies the prior review's Plan output. Return the pair to implementation; do not review it until implementation makes it `review-ready`.
|
|
- A PASS task move is the last review-owned filesystem mutation. After the move, the runtime may idempotently replay the completion event from the archived `complete.log` Finalization metadata; the review model does not reopen or mutate the moved task.
|
|
|
|
## Step 2 - Load Context
|
|
|
|
Count `agent-task/{task_name}/code_review_*.log` in the selected active task directory:
|
|
|
|
- `0`: first review. Read the active review file, active plan file, every planned source file, related tests, and files importing/imported by changed files up to 2 levels deep.
|
|
- `>=1`: follow-up review. Start with `git diff`, `git diff --cached`, and `git log --oneline -5`, then expand to related callers, implementers, tests, and any planned files missing from the diff.
|
|
|
|
The diff is the starting point, not the boundary. Follow behavior and API connections far enough to judge correctness.
|
|
|
|
If the active plan or review file contains `Agent UI Completion`, also read `agent-ops/rules/common/rules-agent-ui.md` and the listed active agent-ui documents. Do not read `agent-ui/definition/archive/**` or `agent-ui/archive/user-review/**` unless the review explicitly cites those archive paths.
|
|
|
|
## Step 3 - Pre-Review Checklist
|
|
|
|
Before writing the verdict:
|
|
|
|
- Compare actual source files against every planned checklist item.
|
|
- For an indexed task directory, require its basename to match `NN_{subtask_name}` or `NN+PP[,QQ...]_{subtask_name}` and every encoded predecessor to be lower than `NN`. A malformed name is a Required orchestration finding and can never PASS.
|
|
- For an indexed task directory with `+PP[,QQ...]`, require one unambiguous matching predecessor `complete.log` for every encoded index before review can PASS. Missing dependency evidence is a Required orchestration finding; the implementation worker never supplies or repairs it.
|
|
- Compare the plan `구현 체크리스트` and review stub `구현 체크리스트`. Do not edit either checklist to repair drift; when correction is needed, record it as a WARN/FAIL finding.
|
|
- When the active artifacts have `Roadmap Targets`, check whether the referenced Milestone has `SDD: 필요`. When it does, read only that Milestone and its SDD, compare implementation evidence against the SDD Acceptance Scenarios/Evidence Map for the targeted task ids, and fail completeness or verification trust when evidence is insufficient. Do not require a separate SDD target section.
|
|
- Record obvious non-behavioral source nits such as typos, stale comments, docs, or formatting. Do not edit source/test files during review; Nit-only findings may follow the PASS rule below.
|
|
- If a checklist item contains integrated verification for a feature, treat that feature item as incomplete until both implementation evidence and the matching verification output are present. Do not accept a separate unchecked completion-criteria item as a substitute.
|
|
- Confirm the implementation marked the matching checklist items in the active review file, including the final mandatory `CODE_REVIEW-*-G??.md` completion item. Do not mark or rewrite implementation-owned items on its behalf; classify missing or inconsistent entries by their effect on completeness and verification trust.
|
|
- Verify that the PLAN `plan-model` metadata (or the legacy `역할 모델 > Plan` field) matches the review ledger and that the review stub records the Implementation identity. Require runtime to record the Review identity before verdict; filled identities are immutable. Reject review start unless all three are pairwise different, and do not infer identity from lane/G or agent name.
|
|
- If the active plan or review file contains `Agent UI Completion`, verify the implementation-owned fields are filled before PASS: listed agent-ui docs exist, actual code evidence paths exist, requested status updates are explicit, and implementation verification evidence is present. Missing or untrusted evidence is a completeness or verification-trust issue.
|
|
- Treat review artifact gaps as failures only when they prevent judging implementation correctness, tests, contracts, or verification trust.
|
|
- Read `사용자 리뷰 요청`. If filled, validate the exact linked Milestone decision and blocker evidence; reject non-lock requests.
|
|
- Grep renamed/removed symbols for stale references.
|
|
- Confirm every required test exists, name matches, and assertions are meaningful.
|
|
- Cross-check claimed verification output in the active review file against actual code and project commands.
|
|
- For follow-up reviews, compare diff against the plan and scan for unplanned changes, debug prints, dead code, TODOs, formatting-only noise, and unrelated edits.
|
|
|
|
## Step 4 - Append Verdict
|
|
|
|
Append `코드리뷰 결과` to the active `CODE_REVIEW-*-G??.md`.
|
|
|
|
Before appending `PASS`, if all other PASS conditions are met and the active review file contains `Agent UI Completion`, perform the review-pass status update first: update the listed agent-ui docs to `구현됨`, add actual code evidence, run `validate-agent-ui`, and mark the section's review-agent-owned finalization items. If this update or validation fails, do not append `PASS`; classify and record it as a normal WARN/FAIL finding.
|
|
|
|
Required fields:
|
|
|
|
- `종합 판정`: exactly `PASS`, `WARN`, or `FAIL`.
|
|
- `차원별 평가`: Pass/Warn/Fail for correctness, completeness, test coverage, API contract, code quality, implementation deviation, verification trust. If SDD Evidence Map applies through `Roadmap Targets`, also include spec conformance.
|
|
- `발견된 문제`: `없음`, or bullets using `Required`, `Suggested`, or `Nit` with `file:line` and a concrete fix.
|
|
- `다음 단계`: keep only the matching PASS, WARN/FAIL follow-up, or USER_REVIEW line.
|
|
|
|
Do not perform archive or terminal-output mutations during Step 4. Step 5 onward owns finalization.
|
|
|
|
Severity semantics:
|
|
|
|
| Verdict | Meaning | Follow-up plan |
|
|
|---------|---------|----------------|
|
|
| `PASS` | No Required/Suggested issues. Nit-only findings may still PASS. | No |
|
|
| `WARN` | One or more Suggested issues, zero Required. | Follow-up pair via a different plan model; a valid gate is normally `FAIL` |
|
|
| `FAIL` | One or more Required issues, including a valid Milestone-lock request. | Follow-up pair via a different plan model, except a valid `USER_REVIEW.md` gate |
|
|
|
|
Issue severity:
|
|
|
|
- `Required`: correctness, API contract, missing required test, missing integrated verification, plan-completeness issue, review artifact gaps that prevent judging implementation quality, or a valid selected Milestone-lock decision that blocks implementation.
|
|
- `Suggested`: useful improvement that should enter the loop but does not block correctness.
|
|
- `Nit`: tiny cleanup; record it without editing implementation-owned files or forcing WARN.
|
|
|
|
Verdict consistency:
|
|
|
|
- `PASS` requires all dimensions to be Pass and no Required/Suggested issues. Nit-only findings may still PASS only when every dimension remains Pass.
|
|
- Any Fail dimension or any Required issue forces `FAIL`.
|
|
- Any Warn dimension or any Suggested issue forces `WARN`, unless the only findings are explicitly Nit and every dimension remains Pass.
|
|
|
|
## Step 5 - Archive Active Files
|
|
|
|
- For a valid user-review gate, render `USER_REVIEW.md` in memory before renaming.
|
|
- Ensure `.gitignore` exposes `agent-task/**/*.md` and `agent-task/**/*.log`.
|
|
- Derive each archive suffix as `0` when no matching log exists, otherwise `max(existing numeric suffix) + 1`; combine it with that active file's own lane/grade.
|
|
- For an uninterrupted archive, compute and preflight both destinations before the first rename and require both to be absent.
|
|
- For partial recovery, identify the archived counterpart by matching `task` and `plan` headers. Treat its actual path as canonical, derive the missing member's destination from the remaining active member's own lane/grade and the current matching log suffixes, require that destination to be absent, and archive only the missing member. Do not invent or rename the existing counterpart.
|
|
- The verdict, user-review gate, task/plan identity, and forbidden Plan identities remain readable from the active or archived review. Derive the post-archive branch from those artifacts after each rename; do not depend on an in-memory value surviving restart.
|
|
- Rename the review, then the plan. Verify no active pair remains.
|
|
|
|
## Step 6 - Post-Review Actions
|
|
|
|
For `PASS`, after both pair logs exist, select one completion timestamp and collision-safe final task archive path, then write `agent-task/{task_name}/complete.log`. If a `USER_REVIEW.md` stop is resolved as complete/PASS, reuse its fixed resolution timestamp and select the collision-safe path when writing `complete.log`. Write and verify `complete.log` before moving that task. On recovery, an existing `complete.log` is canonical; do not select a new timestamp or path.
|
|
|
|
Complete log template:
|
|
|
|
- Template path: `agent-ops/skills/common/code-review/templates/complete-log-template.md`
|
|
- Copy the template's section order and fill every placeholder from the archived plan/review logs and final verdict.
|
|
- Do not leave placeholders in `complete.log`.
|
|
- Fill the required `Finalization` section with the exact template keys: original `origin-task`, collision-safe `final-archive-path`, `task-group`, Milestone slug or `none`, final archived plan/review paths, and roadmap-completion Task ids or `none`. These fields are the runtime's idempotent completion-event input and the final-move recovery source.
|
|
- If the task did not close through `USER_REVIEW.md`, remove the optional user-review row from the `루프 이력` table.
|
|
- If the archived plan or review log contains `Roadmap Targets`, copy it into `complete.log` as `Roadmap Completion`. Include the Milestone path, completed Task ids, archived plan/review log paths, and verification evidence. If there is no `Roadmap Targets` section, remove the optional `Roadmap Completion` template section entirely and do not invent roadmap targets.
|
|
- If the archived review log contains `Agent UI Completion`, copy the completed evidence into `complete.log`. If there is no `Agent UI Completion` section, remove the optional complete-log section entirely.
|
|
- Use `없음` for empty `잔여 Nit` or `후속 작업`.
|
|
- A PASS `complete.log` must not contain unresolved Required or Suggested issues. Nit-only leftovers may be recorded under `잔여 Nit`.
|
|
|
|
For `WARN` or `FAIL` after archive:
|
|
|
|
- If the user-review gate triggered, write and verify `USER_REVIEW.md`.
|
|
- Otherwise dispatch a different plan model with the archived plan/review paths and wait for its readiness result. Require one new routed PLAN/CODE_REVIEW pair at the same task path and verify `implementation-ready` or `dependency-waiting`. The review model must not write that pair or edit product source/tests.
|
|
- Stop before implementation.
|
|
|
|
If the task group is `m-<milestone-slug>` and the user-review gate triggered, prepare a blocked-on-user-review report; do not emit PASS completion metadata and do not call `update-roadmap`.
|
|
|
|
## Step 7 - Finalize, Move PASS Task, And Report
|
|
|
|
After Step 6:
|
|
|
|
- For `PASS`, read the exact task archive path and completion-event fields back from `complete.log`; do not rely on in-memory copies.
|
|
- Before a PASS move, run the ignore check and prepare the branch report from the same `complete.log` metadata. The move must be the only remaining review-owned filesystem mutation.
|
|
- Create the parent of the exact task archive path and preflight the destination again. If it is newly occupied by non-matching evidence while the active task still exists, select the next collision-free suffix, replace every affected old final-archive prefix in `complete.log` including `Finalization` and `Roadmap Completion` evidence paths, re-read and revalidate the whole file, then preflight that new exact path.
|
|
- Move the selected active task directory to that exact path as the final review-owned filesystem mutation. Do not open or update the moved review log, `complete.log`, or any other file after the move.
|
|
- If verdict is `PASS` and `{task_group}` matches `m-<milestone-slug>`, do not resolve the roadmap target and do not call `update-roadmap`. Before the task archive move, verify the `complete.log` Finalization metadata contains `origin-task=agent-task/{task_name}`, `task-group={task_group}`, `milestone-slug=<milestone-slug>`, final archive path, archived plan/review log paths, and `roadmap-completion=<Task ids from complete.log or none>`.
|
|
- The runtime consumes that completion event, checks current state, and calls `update-roadmap` if needed. `update-roadmap` only checks Milestone Task ids when `complete.log` contains `Roadmap Completion`; if the section is absent, roadmap Task completion is a no-op even for `m-*` task groups.
|
|
- If verdict is `PASS` and the archived review log contains `Agent UI Completion`, ensure the listed agent-ui status/evidence update and `validate-agent-ui` check are complete before writing or finalizing `complete.log`. Do not defer this to runtime or Milestone completion.
|
|
- `WARN` and `FAIL` do not update the roadmap Milestone. The dispatched plan model keeps the same task path.
|
|
- `USER_REVIEW` does not update the roadmap Milestone and does not produce PASS completion metadata. Keep the active task directory in place with `USER_REVIEW.md` and archived plan/review logs until the linked Milestone lock decision is resolved.
|
|
- If `USER_REVIEW.md` is later resolved as complete/PASS by linked Milestone decision and evidence, write `complete.log`, move the task directory to archive, and report `m-*` PASS completion metadata just like a normal `PASS`.
|
|
- For `WARN` or `FAIL`, run `git check-ignore -q --` on generated logs and terminal files after the follow-up pair or `USER_REVIEW.md` exists.
|
|
- For `PASS`, perform the same ignore check before the final move. For user-review-resolved PASS, also verify the active task contains the resolved `USER_REVIEW.md`, `complete.log`, and existing archived pair before moving.
|
|
- If any required artifact is missing, finish the archive, follow-up pair handoff, `complete.log`, final-path metadata, or `USER_REVIEW.md` write first.
|
|
- Report only after the selected artifact and final paths are verified. A PASS report is read-only after the final move.
|
|
|
|
Report exactly one output: follow-up plan pair, `USER_REVIEW.md`, or `complete.log`. Include the relevant paths.
|
|
|
|
## Review Dimensions
|
|
|
|
| Dimension | Check |
|
|
|-----------|-------|
|
|
| Correctness | Logic, edge cases, concurrency, errors |
|
|
| Completeness | Planned implementation/verification items are done; implementation-owned artifact drift is classified and routed without reviewer edits |
|
|
| Test coverage | Required tests present and meaningful |
|
|
| API contract | Call sites, compatibility, docs |
|
|
| Code quality | No debug prints, dead code, leftover TODOs |
|
|
| Plan deviation | Deviations justified, no unrelated risk |
|
|
| Verification trust | Reported output matches actual code |
|
|
|
|
## Quality Rules
|
|
|
|
- Lead with findings; use specific `file:line`.
|
|
- Provide a concrete fix for every Required issue.
|
|
- Name exact stale symbols or missing tests.
|
|
- Do not write vague praise or style opinions without a rule.
|
|
- Every dimension gets Pass/Warn/Fail.
|
|
- For verification-trust findings, specify deterministic commands, for example `rg --sort path`, and forbid repo-local tool artifacts.
|
|
|
|
## Final Checklist
|
|
|
|
- Exactly one next-state outcome was selected: a follow-up PLAN/CODE_REVIEW pair, `USER_REVIEW.md`, or `complete.log`.
|
|
- `{current_review_archive_name}` exists with the verdict appended and was derived from the archived active review's own route.
|
|
- `{current_plan_archive_name}` exists and was derived from the archived active plan's own route.
|
|
- `.gitignore` unignores `agent-task/**/*.md` and `agent-task/**/*.log` and ignores local `agent-roadmap/current.md`; generated task artifacts are not ignored by `git check-ignore`.
|
|
- No active `.md` files remain after PASS or user-review-resolved PASS.
|
|
- PASS or user-review-resolved PASS: `complete.log` contains the required Finalization metadata, then task directory moved under `agent-task/archive/YYYY/MM/` as the final filesystem mutation with task-group path preserved for split work.
|
|
- PASS milestone task group: `m-<milestone-slug>` completion event metadata is prepared for runtime; roadmap was not modified by code-review.
|
|
- PASS with `Roadmap Targets`: `complete.log` contains `Roadmap Completion` with Milestone path, Task ids, archived plan/review evidence, and verification evidence.
|
|
- PASS without `Roadmap Targets`: `complete.log` omits `Roadmap Completion` and prepared metadata says `roadmap-completion=none`.
|
|
- PASS with `Agent UI Completion`: listed agent-ui docs were updated to `구현됨` with code evidence, `validate-agent-ui` passed, and `complete.log` contains `Agent UI Completion`.
|
|
- PASS without `Agent UI Completion`: complete.log omits `Agent UI Completion`.
|
|
- WARN/FAIL without user-review gate: a different plan model wrote one follow-up pair, the reviewer did not plan or implement, and the pair has an `implementation-ready` or `dependency-waiting` result.
|
|
- Follow-up review stubs include the `사용자 리뷰 요청` section from `agent-ops/skills/common/_templates/implementation-user-review-request-section.md` and forbid direct user prompts during implementation.
|
|
- USER_REVIEW: `USER_REVIEW.md` exists from template, no active `PLAN-*.md` or `CODE_REVIEW-*.md` remains, and no `complete.log` was written.
|
|
- Implementation-requested USER_REVIEW: archived review log preserves the filled `사용자 리뷰 요청` evidence and the generated `USER_REVIEW.md` records the exact linked Milestone decision needed.
|
|
- USER_REVIEW resolved as PASS: archived task contains both resolved `USER_REVIEW.md` and `complete.log`.
|
|
- USER_REVIEW resolved to more implementation: `USER_REVIEW.md` was archived to a `RESOLVED`/`REPLAN` `user_review_N.log` that references its archived pair and forbidden identities, a different plan model wrote one follow-up pair from that evidence, and the pair has an `implementation-ready` or `dependency-waiting` result.
|