From ad4c6a7c6e72f18f882e5d6c372b046f0bcedace Mon Sep 17 00:00:00 2001 From: pbean Date: Sun, 21 Jun 2026 20:57:57 -0700 Subject: [PATCH] fix(bmm): address automation-mode review comments on quick-dev/code-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the augmentcode review findings on PR #2494, tightening the BMAD_AUTO_MODE contract so unattended runs behave consistently: - code-review automation-mode.md: harmonize the result.json `clean` definition with rule 8 (zero actionable findings AND spec status set to `done`), and correct the `dismissed` count description (recorded to the Review Ledger, not dropped). - code-review step-04 §1: the clean shortcut now keys off actionable findings only and, in auto mode, still writes the Review Ledger when findings were dismissed before jumping to §6 — otherwise set-aside dismissals never reach the ledger and the next cycle re-litigates them. - code-review: give explicit relative paths to bmad-quick-dev's deferred-work-format.md (rule 7 and step-04) so unattended agents can locate it. - code-review step-01: ground the auto-mode note in rule 1 (no HALT anywhere in the step) instead of enumerating a partial skip list. - quick-dev deferred-work-format.md: drop the dead `bmad-auto-sweep/migration-mode.md` reference (that file ships with the orchestrator, not BMAD-METHOD); describe the sweep migration as an external orchestrator-side process. Co-Authored-By: Claude Opus 4.8 --- .../4-implementation/bmad-code-review/automation-mode.md | 6 +++--- .../bmad-code-review/steps/step-01-gather-context.md | 2 +- .../bmad-code-review/steps/step-04-present.md | 6 ++++-- .../4-implementation/bmad-quick-dev/deferred-work-format.md | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/bmm-skills/4-implementation/bmad-code-review/automation-mode.md b/src/bmm-skills/4-implementation/bmad-code-review/automation-mode.md index 70c2b17d5..5bc7cbf0a 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/automation-mode.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/automation-mode.md @@ -17,10 +17,10 @@ this workflow. ```json { "workflow": "code-review", - "clean": , + "clean": , "patched": , "deferred": , - "dismissed": , + "dismissed": , "escalations": [{"type": "", "severity": "CRITICAL|PREFERENCE", "detail": ""}] } @@ -67,7 +67,7 @@ this workflow. 7. **Act** (step-04): write findings to the spec file as usual; apply EVERY `patch` finding without asking; append `defer` findings to the deferred-work file following the format in the sibling `bmad-quick-dev` - skill's `deferred-work-format.md` (same directory conventions); + skill's deferred-work format file at `../bmad-quick-dev/deferred-work-format.md`; skip the "Next steps" menu entirely. 8. **Status updates** (step-04 section 6) run exactly as written: spec status (frontmatter `status:`) and sprint-status sync. diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-01-gather-context.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-01-gather-context.md index d5a6c0bd5..b84b6e9d8 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-01-gather-context.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-01-gather-context.md @@ -12,7 +12,7 @@ story_key: '' # set at runtime when discovered from sprint status - 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. -- If `{auto_mode}`: the invocation argument is the spec file. Apply automation-mode.md rule 2 (target/baseline/story-key), rule 3 (diff source), and rule 4 (no chunk question), then skip instruction 6's question and the CHECKPOINT — go straight to NEXT. +- If `{auto_mode}`: do NOT HALT or ask anywhere in this step (automation-mode.md rule 1). The invocation argument is the spec file — apply rule 2 (target/baseline/story-key; this skips the instruction-1 cascade and instruction 2's ask), rule 3 (diff source), and rule 4 (no chunk question); skip instruction 4's spec-context question, instruction 6's question, and the CHECKPOINT — go straight to NEXT. ## INSTRUCTIONS diff --git a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md index 1fbbb444b..8c57cf849 100644 --- a/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md +++ b/src/bmm-skills/4-implementation/bmad-code-review/steps/step-04-present.md @@ -24,7 +24,9 @@ If `{auto_mode}`, run this step with these substitutions (see automation-mode.md ### 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). +If no **actionable** findings remain after triage — no `decision-needed`, `patch`, or `defer` findings (every finding was dismissed, or none were raised): state that and proceed to section 6 (Sprint Status Update). Set-aside dismissed findings do not count as "remaining". + +**If `{auto_mode}`:** when any findings were dismissed, do NOT skip straight to section 6 — first perform section 2's Review Ledger append (the `#### Review Ledger ({date})` subsection) so the next cycle's reviewers do not re-litigate them, then proceed to section 6. ### 2. Write findings to the story file @@ -41,7 +43,7 @@ If `{spec_file}` exists and contains a Tasks/Subtasks section, append a `### Rev 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. -**If `{auto_mode}`:** instead of the heading-based append above, append each `defer` finding to `{deferred_work_file}` as a `DW-` entry following the format and dedupe rule in the sibling `bmad-quick-dev` skill's `deferred-work-format.md`. Use `origin: code review of , {date}`, and set the entry's `severity:` from the finding's own severity. Then append a `#### Review Ledger ({date})` subsection to `{spec_file}` recording every triaged finding on one line each — `: [<location>] — <one-line reason>` — including the dismissed ones set aside in step-03. The ledger is append-only across review cycles; it is what stops the next cycle's fresh reviewers from re-litigating findings that were already adjudicated. +**If `{auto_mode}`:** instead of the heading-based append above, append each `defer` finding to `{deferred_work_file}` as a `DW-<seq>` entry following the format and dedupe rule in the sibling `bmad-quick-dev` skill's deferred-work format file at `../../bmad-quick-dev/deferred-work-format.md`. Use `origin: code review of <spec basename>, {date}`, and set the entry's `severity:` from the finding's own severity. Then append a `#### Review Ledger ({date})` subsection to `{spec_file}` recording every triaged finding on one line each — `<verdict>: <title> [<location>] — <one-line reason>` — including the dismissed ones set aside in step-03. The ledger is append-only across review cycles; it is what stops the next cycle's fresh reviewers from re-litigating findings that were already adjudicated. ### 3. Present summary 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 0201bf6ff..e1a902c64 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 @@ -5,9 +5,9 @@ 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 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 session — see -`bmad-auto-sweep/migration-mode.md`; the TUI displays such legacy items -read-only until that happens.) +entries by a `bmad-auto sweep` migration run — an orchestrator-side process +external to this skill; the TUI displays such legacy items read-only until that +happens.) ## Before appending: dedupe check