src/bmm-skills/ship/bmad-build-auto/step-04-review.md
{{.communication_language}}, tailored to {{.user_skill_level}}. Write files in {{.document_output_language}}.Change {spec_file} status to in-review in the frontmatter before continuing.
Read {baseline_revision} from {spec_file} frontmatter. If {baseline_revision} is missing or NO_VCS, use best effort to determine what changed. Otherwise use the repository's version-control tooling to rewrite {diff_file} — the temp file staged in step-03, or a uniquely-named file in the system temp directory when this run has none — with a unified diff of all changes since {baseline_revision}, untracked files included. The review layers read that file; the diff text is never pasted into their prompts.
Set {claims_file} = {spec_file}. The spec is the change's own account of itself, and it goes to the edge-case layer alone — as a path, so that layer reads it only after its own tracing and the other layers never see it at all.
Writing {diff_file} is the only change this section makes. Do NOT git add anything.
Runtime placeholders: {diff_file} is the diff staged above and {claims_file} the narrative staged with it — both paths, substituted absolute so a layer can read them; a launch prompt never carries diff text. {verbatim_intent} is the invocation intent exactly as this run received it at step-01; if the run started from an existing spec file rather than a fresh intent, it is the spec's <intent-contract> block instead. Before launching a layer, expand its skill-root placeholder to this skill's absolute installed directory; never leave that placeholder unresolved in a child prompt.
Announce skipped layers first, then launch every active layer before handling any layer's result. Try running all active layers simultaneously: substitute the runtime placeholders (e.g. {diff_file}) into each layer's instruction. When an instruction launches a reviewer subagent, launch that child with the prompt text after placeholder substitution; do not load the reviewer instruction file yourself. For any other customized instruction, execute it as written. Parallel means several blocking calls awaited together in this turn — never backgrounded or detached, never ending the turn to await results (see workflow.md → Subagents). Spawn every reviewer subagent before reading or reacting to any of their output; begin collection and triage only once all are launched.
{workflow.review_layers}
Once every layer has reported — and not before — render a verdict on each finding, ahead of any deduplication or grouping. Disregard any severity a reviewing subagent assigned — they lack the context to grade.
If ## Review Triage Log already has rows — a loopback, a resumed review, or a follow-up pass on a done spec — check each finding against them first. Same location and same claim as a logged row, and the code there still reads as the row describes: keep the row's verdict and route, write the row again with carried in front of the evidence, skip verification, and never patch or defer it again. Verify everything else as below.
For each finding:
A gap finding from the verification-gap layer arrives pre-verified — that layer's evidence rules made it read the tests and run the searches it cites, and triage trusts the claim as filed. Skip verification, render the verdict from the filed evidence, and weigh its filed disposition when routing. Its Other findings are verified like everything else.
Verify the finding's claim. At the cited file and line, does the bad outcome the reviewer describes actually occur? Read beyond the changed lines — follow callers, guards upstream, etc — until you can answer yes or no. A different finding about nearby code does not settle this one. Judge whether the problem is real, not whether the proposed fix is plausible. Code that loudly fails on a situation you never showed the program can reach is correct behavior, not a defect.
Render exactly one verdict from what verification established — the verdict is the whole triage decision; there is no separate keep-or-dismiss.
high (intolerable), medium (tolerable), low (cosmetic or negligible) — the bad outcome is real. Assign severity by how much it hurts end users or developers. For developer-only problems (inconsistent design, eroded invariants, duplicated sources of truth), name where it will cause trouble — which caller will diverge, which rule will break. A vague "this is messy" with no named harm is not a severity grade; use false or maybe-false instead. When the harm is real but you cannot tell how bad, pick the higher grade.false — you checked, and the bad outcome does not happen at the cited location. Write what disproves this specific claim. A true fact about nearby code that does not disprove the claim does not count.maybe-false — you could not tell whether the bad outcome happens. Write what you would need to check to find out. Use this only when the diff and surrounding code leave the question open; when they are enough to decide, pick high, medium, low, or false.Every finding gets one row in the triage log below — verdict plus its evidence in a sentence or two; never drop, merge, or silently skip one.
Reject false findings on their refutation.
Reject low findings when it is unlikely that users or developers would meet the defect in everyday use (judged plainly — no proof needed) and the fix is more than a direct correction or deletion — adding guards, branches, parameters, or other complexity.
Out of scope: reject or defer a finding as out of scope only when the intent itself excludes it — not because the spec's scope section, the plan, or the shape of the diff says so. If only those would exclude it, keep the finding: the spec or plan drew the line somewhere the intent did not, so it routes to intent_gap or bad_spec, never to patch or defer.
Reject any finding whose fix is to edit this build's spec.
All remaining findings continue to grouping.
Group the survivors by shared root cause — two findings belong in one entry only when the same defect produced both. Same location alone is not a shared root cause, and neither is a shared fix. An entry carries every member's verified bad outcome and the highest verdict among them (high > medium > low > maybe-false).
Route each entry into exactly one triage category. A group that includes verified high, medium, or low members routes by its highest such verdict — not to defer just because a member is maybe-false. The first three are this story's problem — caused or exposed by the current change. The last is not this story's problem.
maybe-false and the claim, if true, would be medium or high — record that severity marked unverified, plus what would settle it (if it would only be low, reject it with the same note); or any entry whose fix edits agent-context files (CLAUDE.md, AGENTS.md, rules, etc).Append a new entry to the ## Review Triage Log section in {spec_file}, in this format:
### {date} — Review pass
- verdicts: <total> findings — high <N>, medium <N>, low <N>, false <N>, maybe-false <N>
- findings:
- `[verdict]` `[intent_gap|bad_spec|patch|defer|reject]` <finding summary> — <evidence: the refutation for false, what would settle it for maybe-false, the action taken for patches, why a rejected low was not worth fixing>
Where {date} is the current system date. One row per finding from every layer, in the order the layers reported them; <total> must equal the number of findings the layers reported — a finding missing from the log is a triage failure. Members of a grouped entry keep their own rows and share the route.
Process entries in cascading order. If intent_gap exists, lower entries are moot; follow the intent_gap branch below. If bad_spec exists, lower entries are moot since code will be re-derived. If neither exists, process patch and defer normally. Before each bad_spec loopback, read {spec_file} frontmatter review_loop_iteration (missing means 0), increment it by 1, and write it back. If it exceeds 5, append the triage-log entry for this pass, then HALT with status blocked and blocking condition review repair loop exceeded 5 iterations (non-convergence).
intent_gap — Root cause is inside <intent-contract>. Save the attempted change as a patch file in {{.implementation_artifacts}} and reference it from the triage-log entry, then revert code changes. Append the triage-log entry for this pass, then HALT with status blocked, blocking condition intent gap, and include the unresolved questions and the saved patch path.
bad_spec — Root cause is outside <intent-contract>. Do not modify content inside <intent-contract>. Before reverting code: extract KEEP instructions for positive preservation (what worked well and must survive re-derivation). Revert code changes. Read the ## Spec Change Log in {spec_file} and strictly respect all logged constraints when amending the sections outside <intent-contract> that contain the root cause. Append a new change-log entry recording: the triggering finding, what was amended, the known-bad state avoided, and the KEEP instructions. Append the triage-log entry for this pass, recording in each bad_spec row the amendment it triggered. Read fully and follow [[bmad-snapshot:step-03-implement.md]] to re-derive the code, then this step will run again.
patch — Auto-fix. These are the only findings that survive loopbacks. Re-engage the step-03 implementation subagent — the same one, addressed by the name or id its launch returned; a fresh launch is not re-engagement. Send it one message, exactly this, with the findings filled in:
Review of your implementation found problems. Fix each one below with the smallest change that does the job.
Run only the tests that cover the files you edit — nothing wider. Full verification runs on my side after you return. Reply with what you changed.
- <file> — <what is wrong> — <what the smallest fix must do>
If it cannot be continued, apply the patches yourself. Then re-run the commands in {spec_file}'s ## Verification section (or perform its manual checks); if verification fails and the failure cannot be fixed, HALT with status blocked and blocking condition patch verification failed. Rewrite {diff_file} so it reflects the patched tree. Append the triage-log entry for this pass, recording in each patched row the fix applied.
defer — Update the single deferred list in {spec_file} frontmatter. If the field is absent (including on specs created before this field existed), add it once as an empty list. If it is deferred: [], replace that empty value when adding the first item; otherwise append to the existing list. Preserve every existing item, do not look for duplicates, and never add a second deferred: key. Serialize free-form values as YAML block scalars so characters such as :, #, quotes, and line breaks remain data. Each item uses this shape:
deferred:
- summary: >-
<one sentence>
evidence: |-
<why this is real; for a maybe-false finding, what evidence would settle it>
location: >- # optional — file:line or component
src/foo.py:42
severity: medium # optional — high | medium | low; for a maybe-false entry, its if-true grade plus " (unverified)"
After all appends, parse the complete frontmatter as YAML and verify that deferred is one list containing every prior item plus the new items with their intended text. Repair serialization errors before continuing.
Write the following details to {spec_file} under ## Auto Run Result:
false. Count only this pass's entries triaged patch, at entry verdict — never deferred or false ones. On a first pass, true if any patched entry was high, or if two or more medium entries were patched. On a follow-up pass ({followup_pass} = true), true only if this pass patched a high — otherwise the work has converged; patch volume is never grounds. A true names the specific unverified risk under ## Auto Run Result; if none can be named, it is false. Record the patched counts by verdict.Set {spec_file} frontmatter followup_review_recommended from the computation above.
If version control is unavailable, set {spec_file} frontmatter status: done, then proceed to HALT.
If version control is available, write status: done into {spec_file} frontmatter, then:
{spec_file} when it is tracked in that working copy. Keep commits already created during this run. Verify every reviewed-diff file appears in the change set after {baseline_revision} and none remains uncommitted. Do not push.blocked and blocking condition finalization left repository dirty.HALT with status done.