fix(skills): address code-review findings from PR #2007

src/bmm/workflows/4-implementation/bmad-code-review/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: bmad-code-review
-description: 'Perform adversarial code review finding specific issues. Use when the user says "run code review" or "review this code"'
+description: 'Review code changes adversarially using parallel review layers (Blind Hunter, Edge Case Hunter, Acceptance Auditor) with structured triage into actionable categories. Use when the user says "run code review" or "review this code"'
 ---
 
 Follow the instructions in ./workflow.md.

src/bmm/workflows/4-implementation/bmad-code-review/steps/step-01-gather-context.md

@@ -1,6 +1,4 @@
 ---
-name: Gather Context
-description: 'Determine what to review, construct the diff, and load any spec/context documents.'
 diff_output: '' # set at runtime
 spec_file: '' # set at runtime (path or empty)
 review_mode: '' # set at runtime: "full" or "no-spec"
@@ -25,11 +23,11 @@ review_mode: '' # set at runtime: "full" or "no-spec"
    - When multiple phrases match, prefer the most specific match (e.g., "branch diff" over bare "diff").
    - **If a clear match is found:** Announce the detected mode (e.g., "Detected intent: review staged changes only") and proceed directly to constructing `{diff_output}` using the corresponding sub-case from instruction 3. Skip to instruction 4 (spec question).
    - **If no match from invocation text, check sprint tracking.** Look for a sprint status file (`*sprint-status*`) in `{implementation_artifacts}` or `{planning_artifacts}`. If found, scan for any story with status `review`. Handle as follows:
-     - **Exactly one `review` story:** 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, fall through to instruction 2.
+     - **Exactly one `review` story:** 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, fall through to instruction 2.
      - **Multiple `review` stories:** Present them as numbered options alongside a manual choice option. Wait for user selection. Then use the selected story's context to determine the diff source as in the single-story case above, and proceed to instruction 3.
    - **If no match and no sprint tracking:** Fall through to instruction 2.
 
-2. Ask the user: **What do you want to review?** Present these options:
+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)
@@ -40,7 +38,7 @@ review_mode: '' # set at runtime: "full" or "no-spec"
    - 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 the diff is empty (files have no uncommitted changes), ask the user whether to review the full file contents or to specify a different baseline.
+   - 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. Ask the user: **Is there a spec or story file that provides context for these changes?**

src/bmm/workflows/4-implementation/bmad-code-review/steps/step-02-review.md

@@ -1,6 +1,5 @@
 ---
-name: Review
-description: 'Launch parallel adversarial review layers and collect findings.'
+failed_layers: '' # set at runtime: comma-separated list of layers that failed or returned empty
 ---
 
 # Step 2: Review
@@ -23,16 +22,18 @@ description: 'Launch parallel adversarial review layers and collect findings.'
    - **Acceptance Auditor** (only if `{review_mode}` = `"full"`) -- A subagent that receives `{diff_output}`, the content of the file at `{spec_file}`, and any loaded context docs. Its prompt:
      > You are an Acceptance Auditor. Review this diff against the spec and context docs. Check for: violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, contradictions between spec constraints and actual code. Output findings as a markdown list. Each finding: one-line title, which AC/constraint it violates, and evidence from the diff.
 
-2. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, note the failed layer and proceed with findings from the remaining layers. Report the failure to the user in the next step.
+2. **Subagent failure handling**: If any subagent fails, times out, or returns empty results, append the layer name to `{failed_layers}` (comma-separated) and proceed with findings from the remaining layers.
 
-3. **Fallback** (if subagents are not available): Generate prompt files in `{implementation_artifacts}` -- one per active reviewer:
+3. If `{review_mode}` = `"no-spec"`, note to the user: "Acceptance Auditor skipped — no spec file provided."
+
+4. **Fallback** (if subagents are not available): Generate prompt files in `{implementation_artifacts}` -- one per active reviewer:
    - `review-blind-hunter.md` (always)
    - `review-edge-case-hunter.md` (always)
    - `review-acceptance-auditor.md` (only if `{review_mode}` = `"full"`)
 
    HALT. Tell the user to run each prompt in a separate session and paste back findings. When findings are pasted, resume from this point and proceed to step 3.
 
-4. Collect all findings from the completed layers.
+5. Collect all findings from the completed layers.
 
 
 ## NEXT

src/bmm/workflows/4-implementation/bmad-code-review/steps/step-03-triage.md

@@ -1,6 +1,4 @@
 ---
-name: Triage
-description: 'Normalize, deduplicate, and classify all review findings into actionable categories.'
 ---
 
 # Step 3: Triage
@@ -42,9 +40,9 @@ description: 'Normalize, deduplicate, and classify all review findings into acti
 
 4. **Drop** all `reject` findings. Record the reject count for the summary.
 
-5. If zero findings remain after dropping rejects, note clean review.
+5. If `{failed_layers}` is non-empty, report which layers failed before announcing results. If zero findings remain after dropping rejects AND `{failed_layers}` is non-empty, warn the user that the review may be incomplete rather than announcing a clean review.
 
-6. If any review layer failed or returned empty (noted in step 2), report this to the user now.
+6. If zero findings remain after dropping rejects and no layers failed, note clean review.
 
 
 ## NEXT

src/bmm/workflows/4-implementation/bmad-code-review/steps/step-04-present.md

@@ -1,6 +1,4 @@
 ---
-name: Present
-description: 'Present triaged findings grouped by category with actionable recommendations.'
 ---
 
 # Step 4: Present
@@ -33,7 +31,7 @@ description: 'Present triaged findings grouped by category with actionable recom
 4. If clean review (zero findings across all layers after triage): state that N findings were raised but all were classified as noise, or that no findings were raised at all (as applicable).
 
 5. Offer the user next steps (recommendations, not automated actions):
-   - If `patch` findings exist: "You can ask me to apply these patches, or address them manually."
+   - If `patch` findings exist: "These can be addressed in a follow-up implementation pass or manually."
    - If `intent_gap` or `bad_spec` findings exist: "Consider running the planning workflow to clarify intent or amend the spec before continuing."
    - If only `defer` findings remain: "No action needed for this change. Deferred items are noted for future attention."