bmad 6.11
This commit is contained in:
@@ -0,0 +1,87 @@
|
||||
---
|
||||
diff_output: '' # set at runtime
|
||||
spec_file: '' # set at runtime (path or empty)
|
||||
review_mode: '' # set at runtime: "full" or "no-spec"
|
||||
story_key: '' # set at runtime when discovered from sprint status
|
||||
---
|
||||
|
||||
# Step 1: Gather Context
|
||||
|
||||
## RULES
|
||||
|
||||
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
|
||||
- The prompt that triggered this workflow IS the intent — not a hint.
|
||||
- Do not modify any files. This step is read-only.
|
||||
|
||||
## INSTRUCTIONS
|
||||
|
||||
1. **Find the review target.** The conversation context before this skill was triggered IS your starting point — not a blank slate. Check in this order — stop as soon as the review target is identified:
|
||||
|
||||
**Tier 1 — Explicit argument.**
|
||||
Did the user pass a PR, commit SHA, branch, spec file, or diff source this message?
|
||||
- PR reference → resolve to branch/commit via `gh pr view`. If resolution fails, ask for a SHA or branch.
|
||||
- Commit or branch → use directly.
|
||||
- Spec file → set `{spec_file}` to the provided path. Check its frontmatter for `baseline_commit`. If found, use as diff baseline. If not found, continue the cascade (a spec alone does not identify a diff source).
|
||||
- Also scan the argument for diff-mode keywords that narrow the scope:
|
||||
- "staged" / "staged changes" → Staged changes only
|
||||
- "uncommitted" / "working tree" / "all changes" → Uncommitted changes (staged + unstaged)
|
||||
- "branch diff" / "vs main" / "against main" / "compared to <branch>" → Branch diff (extract base branch if mentioned)
|
||||
- "commit range" / "last N commits" / "<from-sha>..<to-sha>" → Specific commit range
|
||||
- "this diff" / "provided diff" / "paste" → User-provided diff (do not match bare "diff" — it appears in other modes)
|
||||
- When multiple keywords match, prefer the most specific (e.g., "branch diff" over bare "diff").
|
||||
|
||||
**Tier 2 — Recent conversation.**
|
||||
Do the last few messages reveal what the user wants to be reviewed? Look for spec paths, commit refs, branches, PRs, or descriptions of a change. Apply the same diff-mode keyword scan and routing as Tier 1.
|
||||
|
||||
**Tier 3 — Sprint tracking.**
|
||||
Look for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for stories with status `review`:
|
||||
- **Exactly one `review` story:** Set `{story_key}` to the story's key (e.g., `1-2-user-auth`). Suggest it: "I found story <story-id> in `review` status. Would you like to review its changes? [Y] Yes / [N] No, let me choose". If confirmed, use the story context to determine the diff source (branch name derived from story slug, or uncommitted changes). If declined, clear `{story_key}` and fall through.
|
||||
- **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. If a story is selected, set `{story_key}` and use its context to determine the diff source. If manual choice is selected, clear `{story_key}` and fall through.
|
||||
- **None:** Fall through.
|
||||
|
||||
**Tier 4 — Current git state.**
|
||||
If version control is unavailable, skip to Tier 5. Otherwise, check the current branch and HEAD. If the branch is not `main` (or the default branch), confirm: "I see HEAD is `<short-sha>` on `<branch>` — do you want to review this branch's changes?" If confirmed, treat as a branch diff against `main`. If declined, fall through.
|
||||
|
||||
**Tier 5 — Ask.**
|
||||
Fall through to instruction 2.
|
||||
|
||||
Never ask extra questions beyond what the cascade prescribes. If a tier above already identified the target, skip the remaining tiers and proceed to instruction 3 (construct diff).
|
||||
|
||||
2. HALT. Ask the user: **What do you want to review?** Present these options:
|
||||
- **Uncommitted changes** (staged + unstaged)
|
||||
- **Staged changes only**
|
||||
- **Branch diff** vs a base branch (ask which base branch)
|
||||
- **Specific commit range** (ask for the range)
|
||||
- **Provided diff or file list** (user pastes or provides a path)
|
||||
|
||||
3. Construct `{diff_output}` from the chosen source.
|
||||
- For **staged changes only**: run `git diff --cached`.
|
||||
- For **uncommitted changes** (staged + unstaged): run `git diff HEAD`.
|
||||
- For **branch diff**: verify the base branch exists before running `git diff`. If it does not exist, HALT and ask the user for a valid branch.
|
||||
- For **commit range**: verify the range resolves. If it does not, HALT and ask the user for a valid range.
|
||||
- For **provided diff**: validate the content is non-empty and parseable as a unified diff. If it is not parseable, HALT and ask the user to provide a valid diff.
|
||||
- For **file list**: validate each path exists in the working tree. Construct `{diff_output}` by running `git diff HEAD -- <path1> <path2> ...`. If any paths are untracked (new files not yet staged), use `git diff --no-index /dev/null <path>` to include them. If the diff is empty (files have no uncommitted changes and are not untracked), ask the user whether to review the full file contents or to specify a different baseline.
|
||||
- After constructing `{diff_output}`, verify it is non-empty regardless of source type. If empty, HALT and tell the user there is nothing to review.
|
||||
|
||||
4. **Set the spec context.**
|
||||
- If the triggering request or recent conversation **explicitly** states there is no spec (e.g. "no spec", "without a spec", "no-spec"): set `{review_mode}` = `"no-spec"` and clear `{spec_file}` (set it to `''`). Do **not** ask for a spec. Do **not** infer no-spec mode merely because the invocation omitted a spec path.
|
||||
- Else if `{spec_file}` is already set (from Tier 1 or Tier 2): verify the file exists and is readable, then set `{review_mode}` = `"full"`.
|
||||
- Else (neither a spec path nor an explicit no-spec declaration is present): ask the user to choose:
|
||||
1. Provide a spec or story file path for context; or
|
||||
2. Continue without a spec.
|
||||
- If the user provides a path: set `{spec_file}` to that path, verify the file exists and is readable, then set `{review_mode}` = `"full"`.
|
||||
- If the user explicitly chooses to continue without a spec: set `{review_mode}` = `"no-spec"`.
|
||||
|
||||
5. If `{review_mode}` = `"full"` and the file at `{spec_file}` has a `context` field in its frontmatter listing additional docs, load each referenced document. Warn the user about any docs that cannot be found.
|
||||
|
||||
6. Sanity check: if `{diff_output}` exceeds approximately 3000 lines, warn the user and offer to chunk the review by file group.
|
||||
- If the user opts to chunk: agree on the first group, narrow `{diff_output}` accordingly, and list the remaining groups for the user to note for follow-up runs.
|
||||
- If the user declines: proceed as-is with the full diff.
|
||||
|
||||
### CHECKPOINT
|
||||
|
||||
Present a summary before proceeding: diff stats (files changed, lines added/removed), `{review_mode}`, and loaded spec/context docs (if any). HALT and wait for user confirmation to proceed.
|
||||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./step-02-review.md`
|
||||
@@ -0,0 +1,32 @@
|
||||
---
|
||||
failed_layers: '' # set at runtime: comma-separated list of layers that failed or returned empty
|
||||
---
|
||||
|
||||
# Step 2: Review
|
||||
|
||||
## RULES
|
||||
|
||||
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
|
||||
- All review subagents must run at the same model capability as the current session.
|
||||
- Run subagents synchronously: launch them together, then wait for all results before continuing.
|
||||
|
||||
## INSTRUCTIONS
|
||||
|
||||
1. The review layers are `{workflow.review_layers}`, resolved during activation.
|
||||
|
||||
2. For each layer in `{workflow.review_layers}`:
|
||||
- `instruction` empty or missing → drop the layer silently (an override disabled it).
|
||||
- `when` condition present and not satisfied by the current context (`{review_mode}`, `{spec_file}`) → drop the layer and tell the user, e.g. "Acceptance Auditor skipped — no spec file provided."
|
||||
- otherwise → the layer is active.
|
||||
|
||||
If no layer is active, HALT with status `blocked` and blocking condition `no active review layers`.
|
||||
|
||||
3. Execute all active layers in parallel wherever their execution methods allow: expand `{skill-root}` in each layer's `instruction` to this skill's absolute installed directory, then substitute the runtime placeholders (`{diff_output}`, `{spec_file}`). For an instruction that 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. Do not leave `{skill-root}` unresolved in a child prompt. If a layer's instruction requires subagents and subagents are not available, for each such layer write under `{implementation_artifacts}` the exact child prompt from that layer's instruction after placeholder substitution (not a path-only pointer), then HALT. Ask the user to run each in a separate session (ideally a different LLM) and paste back the findings. When findings are pasted, treat them as those layers' findings and resume from this point. This is the only allowed parent-side read of a reviewer instruction file.
|
||||
|
||||
4. **Layer failure handling**: If any layer fails, times out, or returns empty results, append the layer's `name` to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
|
||||
|
||||
5. Collect all findings from the completed layers, keeping track of each finding's originating layer `id`.
|
||||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./step-03-triage.md`
|
||||
@@ -0,0 +1,50 @@
|
||||
---
|
||||
---
|
||||
|
||||
# Step 3: Triage
|
||||
|
||||
## RULES
|
||||
|
||||
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
|
||||
|
||||
## INSTRUCTIONS
|
||||
|
||||
1. **Normalize** findings from all layers into a unified list where each finding has:
|
||||
- `id` -- sequential integer
|
||||
- `source` -- the `id` of the layer that produced the finding (e.g., `blind-hunter`), or merged sources joined with `+` (e.g., `blind-hunter+edge-case-hunter`)
|
||||
- `title` -- one-line summary
|
||||
- `detail` -- full description
|
||||
- `location` -- file and line reference (if available)
|
||||
|
||||
2. **Deduplicate.** Deduplicate only findings with the same claim and same required action. If two or more findings meet both conditions, merge them into one:
|
||||
- Use the most specific finding as the base (prefer findings with a precise location over prose-only findings).
|
||||
- Append any unique detail, reasoning, or location references from the other finding(s) into the surviving `detail` field.
|
||||
- Set `source` to the merged sources (e.g., `blind-hunter+edge-case-hunter`).
|
||||
|
||||
3. Then evaluate each remaining finding independently. Do not reject a finding because a related finding was rejected.
|
||||
|
||||
4. **Read the code before rating.** Before assigning severity, open the source at each finding's location and read enough surrounding code to judge reachability -- call sites, guards, and validation that live outside the diff hunk. Do not rate from the diff hunk alone. Severity reflects the real consequence at a real call site, not the worst theoretical reading.
|
||||
|
||||
5. **Assign severity** to each finding by consequence for the artifact's main consumer (software user, document reader, etc).
|
||||
Disregard any severity assigned by a reviewing subagent. Review subagents operate under by-design information asymmetry and do not have enough context to set final severity for this workflow.
|
||||
- `low` -- none or cosmetic
|
||||
- `medium` -- tolerable
|
||||
- `high` -- intolerable
|
||||
|
||||
6. **Route** each finding into exactly one triage bucket:
|
||||
- **decision_needed** -- There is an ambiguous choice that requires human input. The code cannot be correctly patched without knowing the user's intent. Only possible if `{review_mode}` = `"full"`.
|
||||
- **patch** -- Code issue that is fixable without human input. The correct fix is unambiguous.
|
||||
- **defer** -- Pre-existing issue not caused by the current change. Real but not actionable now.
|
||||
- **dismiss** -- Noise, false positive, or handled elsewhere.
|
||||
|
||||
If `{review_mode}` = `"no-spec"` and a finding would otherwise be `decision_needed`, reclassify it as `patch` (if the fix is unambiguous) or `defer` (if not).
|
||||
|
||||
7. **Drop** all `dismiss` findings. Record the dismiss count for the summary.
|
||||
|
||||
8. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping dismissed AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review.
|
||||
|
||||
9. If zero findings remain after triage (all rejected or none raised): state "✅ Clean review — all layers passed." (Step 3 already warned if any review layers failed via `{failed_layers}`.)
|
||||
|
||||
## NEXT
|
||||
|
||||
Read fully and follow `./step-04-present.md`
|
||||
@@ -0,0 +1,132 @@
|
||||
---
|
||||
deferred_work_file: '{implementation_artifacts}/deferred-work.md'
|
||||
---
|
||||
|
||||
# Step 4: Present and Act
|
||||
|
||||
## RULES
|
||||
|
||||
- YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`
|
||||
- When `{spec_file}` is set, always write findings to the story file before offering action choices.
|
||||
- `decision-needed` findings must be resolved before handling `patch` findings.
|
||||
|
||||
## INSTRUCTIONS
|
||||
|
||||
### 1. Clean review shortcut
|
||||
|
||||
If zero findings remain after triage (all dismissed or none raised): state that and proceed to section 6 (Sprint Status Update).
|
||||
|
||||
### 2. Write findings to the story file
|
||||
|
||||
If `{spec_file}` exists and contains a Tasks/Subtasks section, append a `### Review Findings` subsection. Write all findings in this order:
|
||||
|
||||
1. **`decision-needed`** findings (unchecked):
|
||||
`- [ ] [Review][Decision] <Title> — <Detail>`
|
||||
|
||||
2. **`patch`** findings (unchecked):
|
||||
`- [ ] [Review][Patch] <Title> [<file>:<line>]`
|
||||
|
||||
3. **`defer`** findings (checked off, marked deferred):
|
||||
`- [x] [Review][Defer] <Title> [<file>:<line>] — deferred, pre-existing`
|
||||
|
||||
Also append each `defer` finding to `{deferred_work_file}` under a heading `## Deferred from: code review ({date})`. If `{spec_file}` is set, include its basename in the heading (e.g., `code review of story-3.3 (2026-03-18)`). One bullet per finding with description.
|
||||
|
||||
### 3. Present summary
|
||||
|
||||
Announce what was written:
|
||||
|
||||
> **Code review complete.** <D> `decision-needed`, <P> `patch`, <W> `defer`, <R> dismissed as noise.
|
||||
|
||||
If `{spec_file}` is set, add: `Findings written to the review findings section in {spec_file}.`
|
||||
Otherwise add: `Findings are listed above. No story file was provided, so nothing was persisted.`
|
||||
|
||||
### 4. Resolve decision-needed findings
|
||||
|
||||
If `decision_needed` findings exist, present each one with its detail and the options available. The user must decide — the correct fix is ambiguous without their input. Walk through each finding (or batch related ones) and get the user's call. Once resolved, each becomes a `patch`, `defer`, or is dismissed.
|
||||
|
||||
If the user chooses to defer, ask: Quick one-line reason for deferring this item? (helps future reviews): — then append that reason to both the story file bullet and the `{deferred_work_file}` entry.
|
||||
|
||||
**HALT** — I am waiting for your numbered choice. Reply with only the number. Do not proceed until you select an option.
|
||||
|
||||
### 5. Handle `patch` findings
|
||||
|
||||
If `patch` findings exist (including any resolved from step 4), HALT. Ask the user:
|
||||
|
||||
If `{spec_file}` is set, present all three options:
|
||||
|
||||
> **How would you like to handle the `<P>` `patch` findings?**
|
||||
> 1. **Apply every patch** — fix all of them now, no per-finding confirmation. Defer and decision-needed items are not touched.
|
||||
> 2. **Leave as action items** — they are already in the story file
|
||||
> 3. **Walk through each patch** — show details for each before deciding
|
||||
|
||||
If `{spec_file}` is **not** set, present only options 1 and 2 (omit "Leave as action items" — findings were not written to a file):
|
||||
|
||||
> **How would you like to handle the `<P>` `patch` findings?**
|
||||
> 1. **Apply every patch** — fix all of them now, no per-finding confirmation. Defer and decision-needed items are not touched.
|
||||
> 2. **Walk through each patch** — show details for each before deciding
|
||||
|
||||
**HALT** — I am waiting for your numbered choice. Reply with only the number. Do not proceed until you select an option.
|
||||
|
||||
- **Apply every patch**: Apply every patch finding without per-finding confirmation. Do not modify defer or decision-needed items. After all patches are applied, present a summary of changes made. If `{spec_file}` is set, check off the patch items in the story file (leave defer items as-is).
|
||||
- **Leave as action items** (only when `{spec_file}` is set): Done — findings are already written to the story.
|
||||
- **Walk through each patch**: Present each finding with full detail, diff context, and suggested fix. After walkthrough, re-offer the applicable options above.
|
||||
|
||||
**HALT** — I am waiting for your numbered choice. Do not proceed until you select an option.
|
||||
|
||||
**✅ Code review actions complete**
|
||||
|
||||
- Decision-needed resolved: <D>
|
||||
- Patches handled: <P>
|
||||
- Deferred: <W>
|
||||
- Dismissed: <R>
|
||||
|
||||
### 6. Update story status and sync sprint tracking
|
||||
|
||||
Skip this section if `{spec_file}` is not set.
|
||||
|
||||
#### Determine new status based on review outcome
|
||||
|
||||
- If all `decision-needed` and `patch` findings were resolved (fixed or dismissed) AND no unresolved `high`/`medium` findings remain: set `{new_status}` = `done`. Update the story file Status section to `done`.
|
||||
- If `patch` findings were left as action items, or unresolved issues remain: set `{new_status}` = `in-progress`. Update the story file Status section to `in-progress`.
|
||||
|
||||
Save the story file.
|
||||
|
||||
#### Sync sprint-status.yaml
|
||||
|
||||
If `{story_key}` is not set, skip this subsection and note that sprint status was not synced because no story key was available.
|
||||
|
||||
If `{sprint_status}` file exists:
|
||||
|
||||
1. Load the FULL `{sprint_status}` file.
|
||||
2. Find the `development_status` entry matching `{story_key}`.
|
||||
3. If found: update `development_status[{story_key}]` to `{new_status}`. Update `last_updated` to current date. Save the file, preserving ALL comments and structure including STATUS DEFINITIONS.
|
||||
4. If `{story_key}` not found in sprint status: warn the user that the story file was updated but sprint-status sync failed.
|
||||
|
||||
If `{sprint_status}` file does not exist, note that story status was updated in the story file only.
|
||||
|
||||
#### Completion summary
|
||||
|
||||
> **Review Complete!**
|
||||
>
|
||||
> **Story Status:** `{new_status}`
|
||||
> **Issues Fixed:** <fixed_count>
|
||||
> **Action Items Created:** <action_count>
|
||||
> **Deferred:** <W>
|
||||
> **Dismissed:** <R>
|
||||
|
||||
### 7. Next steps
|
||||
|
||||
Present the user with follow-up options:
|
||||
|
||||
> **What would you like to do next?**
|
||||
> 1. **Start the next story** — run `dev-story` to pick up the next `ready-for-dev` story
|
||||
> 2. **Re-run code review** — address findings and review again
|
||||
> 3. **Done** — end the workflow
|
||||
|
||||
**HALT** — I am waiting for your choice. Do not proceed until the user selects an option.
|
||||
|
||||
## On Complete
|
||||
|
||||
Run: `uv run {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow.on_complete`
|
||||
|
||||
If the resolved `workflow.on_complete` is non-empty, follow it as the final terminal instruction before exiting.
|
||||
Reference in New Issue
Block a user