From 0d2863f77fbc89883b08e3fb5a3af4be11d64ce9 Mon Sep 17 00:00:00 2001 From: Alex Verkhovsky Date: Fri, 20 Mar 2026 01:04:51 -0600 Subject: [PATCH] fix: separate subagent launch from skill invocation in code review (#2069) * fix: separate subagent launch from skill invocation in code review The step-02-review prompt fused "invoke skill X" with "in a subagent" into one instruction, causing LLMs to search for a named agent instead of launching a generic subagent that uses the skill. Aligns with the working pattern in quick-dev step-04: upfront gate with inline fallback, and "Invoke via the skill" as a separate concern from subagent setup. Co-Authored-By: Claude Opus 4.6 (1M context) * fix(code-review): address PR review findings on subagent fallback wording Capitalize "Markdown" (proper noun) in Acceptance Auditor prompt and simplify fallback trigger from "context-free subagents" to "subagents" to eliminate ambiguity about when the fallback activates. --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .../bmad-code-review/steps/step-02-review.md | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-02-review.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-02-review.md index 306613014..c262a4971 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-02-review.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-02-review.md @@ -13,27 +13,20 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o ## INSTRUCTIONS -1. Launch parallel subagents. Each subagent gets NO conversation history from this session: +1. If `{review_mode}` = `"no-spec"`, note to the user: "Acceptance Auditor skipped — no spec file provided." - - **Blind Hunter** -- Invoke the `bmad-review-adversarial-general` skill in a subagent. Pass `content` = `{diff_output}` only. No spec, no project access. +2. Launch parallel subagents without conversation context. If subagents are not available, generate prompt files in `{implementation_artifacts}` — one per reviewer role below — and HALT. Ask the user to run each in a separate session (ideally a different LLM) and paste back the findings. When findings are pasted, resume from this point and proceed to step 3. - - **Edge Case Hunter** -- Invoke the `bmad-review-edge-case-hunter` skill in a subagent. Pass `content` = `{diff_output}`. This subagent has read access to the project. + - **Blind Hunter** — receives `{diff_output}` only. No spec, no context docs, no project access. Invoke via the `bmad-review-adversarial-general` skill. - - **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. + - **Edge Case Hunter** — receives `{diff_output}` and read access to the project. Invoke via the `bmad-review-edge-case-hunter` skill. -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. + - **Acceptance Auditor** (only if `{review_mode}` = `"full"`) — 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. -3. If `{review_mode}` = `"no-spec"`, note to the user: "Acceptance Auditor skipped — no spec file provided." +3. **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. -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. - -5. Collect all findings from the completed layers. +4. Collect all findings from the completed layers. ## NEXT