Skip to content

feat(build): reproduce bugs first and verify before review - #3090

Open
DressPD wants to merge 7 commits into
bmad-code-org:devfrom
DressPD:feat/build-reproduce-bug-first
Open

DressPD wants to merge 7 commits into
bmad-code-org:devfrom
DressPD:feat/build-reproduce-bug-first

Conversation

@DressPD

@DressPD DressPD commented Oct 14, 2026 •

Copy link
Copy Markdown

What

bmad-build now reproduces a bug before fixing it, and runs the plan's ## Verification commands itself before review.

Why

  • A bug fix could skip reproduction and be checked only against the new code. bmad-ticket already requires a test that fails on the defect and passes on the fix; bmad-build did not.
  • Nothing forced the plan's Verification commands to run. The oneshot route never mentioned them, and the full route only audited matrix tests, leaving verification to the implementer's own report.

How

  • step-02-plan.md: for a bug fix, put the failing test or command in ## Verification.
  • step-03-implement.md: new Run Verification step before the Matrix Test Audit.
  • step-oneshot.md: run the same commands before review.
  • Failures are fixed, or the run halts and asks the human. No template or route changes.

Testing

  • uv run tools/validate_skills.py --strict
  • uv run tools/validate_file_refs.py --strict
  • uv run --frozen python -B -m pytest

@DressPD DressPD changed the title feat(build): reproduce bugs before fixing them feat(build): reproduce bugs first and verify before review Oct 14, 2026
@DressPD
DressPD marked this pull request as ready for review October 14, 2026 22:50
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

bmad-build now requires bug reproduction during planning and runs the plan’s verification commands before review.

  • Bug plans now include a check that reproduces the defect.
  • Both build routes run their planned checks before review.

Reviews (1) · Last reviewed commit: "feat(build): run plan verification comma..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: bmad-code-org/BMAD-METHOD/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c10672b-dd7f-4e79-9050-aba8c6bc43b4
📥 Commits

Reviewing files that changed from the base of the PR and between 8f0d50e and f309dd9.

📒 Files selected for processing: 3
  • skills/bmad-build/step-02-plan.md
  • skills/bmad-build/step-03-implement.md
  • skills/bmad-build/step-oneshot.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The planning step adds defect reproduction and verification requirements for bug fixes. The implementation and one-shot steps require running planned verification commands, addressing failures, and reporting failures that cannot be resolved.

Changes: Build Verification

Layer / File(s) Summary
Plan defect verification
skills/bmad-build/step-02-plan.md
For bug fixes, the plan must record a reproduced defect and a failing test or command under ## Verification. The fix is complete only when that verification passes.
Run planned verification
skills/bmad-build/step-03-implement.md, skills/bmad-build/step-oneshot.md
Both steps require running and reading the output of every planned verification command, fixing failures, and halting to inform the human if a failure cannot be resolved. The implementation step also requires rewriting and rereading {diff_file} if code changes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Feature

Suggested reviewers: alexeyv

Merge Risk: ⚪ Minimal · up to f309d

The verification section is available in the plan template, and the one-shot review diff reflects changes made during verification. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 4
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the main changes: reproducing bugs before fixes and verifying the plan before review.
Description check Passed The description directly explains the workflow changes, reasons, implementation details, and validation commands.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rubensousa-uw

Copy link
Copy Markdown

Nice addition. One suggestion for the step-02-plan.md line: say what counts as a reproduction. As written, a test that fails for a nearby reason, or a flaky one, still satisfies the rule.

For a bug fix, run a reproduction that fails on the reported symptom before fixing it. Record the command and its pass/fail criterion in the plan's `## Verification`; for intermittent failures, state the repetitions and threshold. If reproduction remains impossible after investigation, record what was tried and the missing evidence, and halt before implementation on either route. The fix is done only when Verification passes.

This stays within the PR's design. Investigation still does not ask the human, and the halt applies to oneshot too, since oneshot skips the Open Questions checkpoint.

The criteria come from Matt Pocock's diagnosing-bugs skill (https://github.com/mattpocock/skills, MIT).

@DressPD

DressPD commented Oct 19, 2026

Copy link
Copy Markdown
Author

Thanks @rubensousa-uw, good catch. A test that fails for a nearby reason, or a flaky one, would have satisfied the old wording.

Taken as suggested, with HALT capitalised to match the rest of the step. step-02-plan.md now requires a reproduction that fails on the reported symptom, with the command and pass/fail criterion (and repetitions and threshold for intermittent failures) recorded in ## Verification. If it can't be reproduced after investigation, the plan records what was tried and the missing evidence, then halts before implementation on both routes. Pushed in 5fd6750.

Credit to Matt Pocock's diagnosing-bugs skill (MIT) for the criteria.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants