appsok/agent-ops/skills/common/code-review/SKILL.md

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.