From d8bd11b715ffd63d407f4608cc8a22d6a1baea95 Mon Sep 17 00:00:00 2001 From: pbean Date: Mon, 22 Jun 2026 10:42:09 -0700 Subject: [PATCH] fix(bmm): address review batch on bmad code-review/quick-dev skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - step-02-review.md: static prefilter now runs BOTH the policy.toml [verify] gates AND the spec's ## Verification commands (deduped), instead of "first match wins". The orchestrator runs only policy.toml itself, so spec-specific checks were otherwise skipped whenever policy.toml existed (augment). - step-03-triage.md: guard the "Clean review — all layers passed" announcement on {failed_layers} being empty, and fix the stale "Step 3 already warned" reference (the warning lives in step 6) — resolves the contradiction with step 6 (augment). - deferred-work-format.md (quick-dev): add the sibling-copy sync note to mirror the bmad-code-review copy (coderabbit). - automation-mode.md: catalogue the spec-write-failure and verification-failure escalation types in the decision table so every emitted type is documented (coderabbit). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../bmad-code-review/steps/step-02-review.md | 12 ++++++++---- .../bmad-code-review/steps/step-03-triage.md | 2 +- .../bmad-quick-dev/automation-mode.md | 2 ++ .../bmad-quick-dev/deferred-work-format.md | 5 ++++- 4 files changed, 15 insertions(+), 6 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 5de5b4817..510131a80 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 @@ -16,10 +16,14 @@ failed_layers: '' # set at runtime: comma-separated list of layers that failed o 0. **Static prefilter** (`{auto_mode}` only — skip entirely in interactive runs). Before any LLM review, run the project's deterministic checks — they are free, precise findings the hunters should not have to - rediscover. Resolve the command list in this order, first match wins: - 1. `[verify] commands` in `{project-root}/.automator/policy.toml` (if the file exists) - 2. the `## Verification` commands in `{spec_file}` (if `{review_mode}` = `"full"`) - 3. none found — skip this instruction silently. + rediscover. Build the command list from BOTH sources below (the orchestrator + runs only the policy.toml gates itself, never the spec's, so the spec's + story-specific checks must run here or nowhere), then run each unique command + once — dedupe identical command strings: + - `[verify] commands` in `{project-root}/.automator/policy.toml` (if the file exists) — the orchestrator's project-wide gates + - the `## Verification` commands in `{spec_file}` (if `{review_mode}` = `"full"`) — story-specific checks + + If neither source yields a command, skip this instruction silently. Record each failing command as a finding with `source: static` (title = the command, detail = the failure output tail). Summarize failures in one line diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-03-triage.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-03-triage.md index b1151f78b..818866131 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-03-triage.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-03-triage.md @@ -62,7 +62,7 @@ 6. 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. -7. 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}`.) +7. If zero findings remain after triage (all rejected or none raised) AND `{failed_layers}` is empty: state "✅ Clean review — all layers passed." (If any layers failed, step 6 already issued the incompleteness warning instead of a clean announcement.) ## NEXT diff --git a/src/bmm-skills/4-implementation/bmad-quick-dev/automation-mode.md b/src/bmm-skills/4-implementation/bmad-quick-dev/automation-mode.md index 196d03a17..d9da35d51 100644 --- a/src/bmm-skills/4-implementation/bmad-quick-dev/automation-mode.md +++ b/src/bmm-skills/4-implementation/bmad-quick-dev/automation-mode.md @@ -109,6 +109,8 @@ overflow choose `[S] Split` per the decision table. | step-01/02 unclear intent after investigation | Escalate `CRITICAL` (`type: intent-gap`). Do not fantasize requirements. | | step-02 token budget exceeded | Choose **[S] Split** (defer secondary scope per `./deferred-work-format.md`). | | step-02 CHECKPOINT 1 | Perform the self-review against the READY FOR DEVELOPMENT standard, fix what it surfaces, then auto-approve: set status `ready-for-dev`, lock the frozen block, continue to step-03. | +| step-02 CHECKPOINT 1 spec preflight fails | If `{spec_file}` is missing or empty after the write, escalate `CRITICAL` (`type: spec-write-failure`) instead of approving. | +| step-auto-finalize verification cannot pass | If a spec `## Verification` command cannot pass without violating the frozen intent, escalate `CRITICAL` (`type: verification-failure`) instead of finalizing. | | step-03 missing/empty spec precondition | Escalate `CRITICAL` (`type: missing-spec`). | | step-04 no sub-agents → "generate prompt files & HALT" | Only reachable when `$BMAD_AUTO_SKIP_REVIEW` is set. Sub-agents are pre-authorized — run the three reviewers inline; never generate prompt files or HALT. | | step-04 `intent_gap` finding (loop back to human) | Revert the code changes, then escalate `CRITICAL` (`type: intent-gap`). Do not infer intent. | diff --git a/src/bmm-skills/4-implementation/bmad-quick-dev/deferred-work-format.md b/src/bmm-skills/4-implementation/bmad-quick-dev/deferred-work-format.md index 9690630a8..58825f0ba 100644 --- a/src/bmm-skills/4-implementation/bmad-quick-dev/deferred-work-format.md +++ b/src/bmm-skills/4-implementation/bmad-quick-dev/deferred-work-format.md @@ -2,7 +2,10 @@ Canonical entry format for `{implementation_artifacts}/deferred-work.md`. Used (in `bmad-auto` automation mode) by bmad-quick-dev (multi-goal splits, -token splits, review defers) and bmad-code-review (defer findings). The file is +token splits, review defers) and bmad-code-review (defer findings). This copy +lives in the bmad-quick-dev skill; bmad-code-review ships a sibling copy of the +same entry format (its surrounding prose differs slightly) — keep the format +itself in sync across the two when it changes. The file is append-only — never rewrite or delete existing entries. (One exception: freeform pre-DW-format content from older projects is rewritten wholesale into canonical entries by a `bmad-auto sweep` migration run — an orchestrator-side process