mirror of
https://github.com/bmad-code-org/BMAD-METHOD.git
synced 2026-08-29 03:44:18 +08:00
Brian
e9f636c1f7
* feat(bmm): rework bmad-retrospective as an evidence-based epic review Replace the persona-meeting simulation with an evidence engine that mines what a completed epic actually left behind — spec, full diff, per-story commits, sprint status, and session logs — instead of simulating a team that "remembers." Every finding carries a source reference. - Five-phase flow: Gather -> Analyze -> Decide -> Close. Analyze derives aggregate views (architecture delta, duplication, god-class growth, pattern divergence, spec reconciliation) and reuses bmad-review for the diff-scope code lenses rather than reimplementing them. - Acceptance verdict (accepted / accepted-with-open-items / rejected); a failing epic can never close as silently accepted. - Team discussion is now opt-in and off by default, delegating to bmad-party-mode seeded with the real findings. - Add deterministic scripts: sprint_status.py (epic detection plus a comment-preserving, force-quoting, validate-and-restore sprint-status.yaml update) and git_evidence.py (range / commit / size-trajectory measurement), with a pytest suite. - The retrospective document is the working artifact: built early, filled per phase, and resumable. Adds a headless mode (-H) with assumption capture. - SKILL.md drops from ~1527 lines to a lean spine plus five references. * fix(bmm): harden retrospective scripts for orchestrator use Address review feedback on the evidence-based retrospective (PR #2612) from CodeRabbit and the bmad-loop maintainer. All changes are additive — no existing JSON field or CLI argument that a consumer reads is renamed or removed. - detect-epic: STORY_RE now matches split-story keys (2-6a-...), aligning with bmad-loop's sprint-status detection; previously such an epic was invisible and a headless run could retro the wrong epic. - sprint_status/git_evidence: argparse failures now emit JSON on the documented stdout contract instead of plain usage text. - sprint_status: guard non-list action_items (clean JSON error, no traceback); _restore reports success/failure so a failed rollback is surfaced, not hidden. - action_items entries carry a stable id and a ref to the retro document, so an orchestrator can dedupe across re-runs and dispatch each sourced finding. - update echoes the acceptance verdict in its JSON; the retro key value stays "done" for existing lifecycle consumers. Retro doc gains machine-readable frontmatter, and -H <epic> is documented as the stable automation interface. - evidence-gathering: derived range now includes the first story commit (<first>^..<last>); aggregate-views drops the size/acceleration overclaim. - Tests: split-story detection, non-list guard, restore reporting, argparse JSON, and id/ref/verdict coverage (13 pass). * docs(bmm): refine retrospective party-mode phrasing * docs(bmm): document the reworked retrospective - Add docs/explanation/retrospective.md — a short feature page: what the skill does, why to run it after an epic, and what to do with its output (retro document, action items, acceptance verdict). - Update the workflow-map retrospective row to reflect the evidence-based review and its actual outputs instead of "Lessons learned." * fix(bmm): validate git_evidence --range as a strict revision range An unvalidated --range reached the git argv unchanged: a value starting with "-" was consumed by git as an option, a single rev logged all history up to it, an existing path was consumed as a pathspec, and a file named like a range (a..b) was silently logged as one — each returning a confident, wrong evidence set with exit 0. Reject anything that is not an explicit REV..REV form — including empty endpoints ("..", "a..", "..b"), which git silently defaults to HEAD, and unstripped values — and terminate the git argv with "--" so a range-shaped name is always parsed as revisions, never as a pathspec. Drop the root-commit special case from evidence-gathering.md: its wording steered into a bare-rev invocation now rejected, the scenario (an epic starting at the repository's first commit) is vanishingly rare, and an agent that ever meets it can still construct a shape-valid range itself. Document the exit-code split (2 invalid args, 1 git failure). Every rejected mode is covered by regression tests; git error-string assertions are pinned to LC_ALL=C. * fix(bmm): harden sprint_status.py input validation and atomic writes Nine findings from the PR #2612 review of the reworked retrospective, all reproduced against the previous behavior: - Pin the emitter indentation so a round-trip stops de-indenting pre-existing, untouched action_items; the sprint-status template now survives an update byte-identical apart from last_updated. - Keep malformed input on the documented JSON-to-stdout contract instead of a traceback: shape-check the root document and development_status on both subcommands, widen the file open from FileNotFoundError to OSError, and report invalid UTF-8 as a JSON error. A non-mapping development_status previously reported ok:true while doing nothing. - Validate comment preservation against the whole file's comment lines rather than only the leading block matched by substring, so a lost mid-file comment fails the write and restores. - Write atomically through a temp file, fsync and os.replace, resolving symlinks and carrying the target's mode across. The rollback goes through the same path: a truncating rewrite that died halfway used to destroy the bytes it was restoring. - Stop inserting an empty development_status mapping, and report retro_key_found: null when --set-retro-done was not passed, reserving true/false for "flag passed, key present/absent". - Include restored:true on every update failure that precedes the write. - Make _slugify Unicode-aware with a content-hash fallback, so non-Latin action text no longer collapses to the literal "item". - Reject an --add-action item whose action is empty or not a string. - Validate --date against MM-DD-YYYY HH:MM and normalize it, since strptime otherwise admits unpadded spellings. Each fix is pinned by a subprocess-level regression test; the suite goes from 18 to 49 tests. * feat(bmm): add action-item status updates to sprint_status.py The update subcommand could only append, and retro-document.md forbids hand-editing sprint-status.yaml, so nothing anywhere could move an action item off open. bmad-sprint-status renders every open and in-progress item on every call, so retrospectives accumulated permanently-open noise. Add --set-action-status, taking a JSON array of transitions for items already in the file. Each entry selects one item by id, or by epic plus exact action text for legacy entries written before ids existed, and gives it one of open, in-progress, done. Selectors resolve against action_items as loaded and strictly before the --add-action append, so an item added in the same run is not addressable in that run. Validation is all-or-nothing and entirely pre-write: a selector matching nothing, matching more than one item, colliding with another entry, or carrying a status outside the vocabulary aborts the whole invocation with restored: true and the file byte-identical. After the write the file is re-parsed and every targeted item is confirmed to carry its new status. Success reports the new action_items_updated count. Rewrite the contradicting paragraph in retro-document.md so the flag is the one sanctioned path to change a status: offer the transitions when the Phase 4 follow-through has evidence an item landed, apply only what the user confirmed, and never touch prior statuses in a headless run. * fix(bmm): measure renames, merges, and binary churn in git_evidence Five ways the measurement was quietly wrong: - Renames emitted an unopenable "src/{a => b}" pseudo-path and split one file's churn across up to three keys; non-ASCII paths arrived octal escaped. Both passes now run with core.quotePath=false and --no-renames, so a rename is an honest delete + add and a path is a real string that opens. - Merge commits emit no numstat rows, so cross-story conflict-resolution churn vanished with nothing saying it had been skipped. The listing pass keeps full topology (per-story attribution is unchanged); a second pass measures merges alone under -m --first-parent --min-parents=2 and reports merge_files, merge_count and merges_measured. Merge churn is never folded into files: a merge's first-parent diff restates the churn of the commits it merged in. - A commit naming two stories attributed to whichever id was passed first. Every matching id is now collected, so a seam commit counts for both stories. commits[].story becomes commits[].stories. - A git failure with empty stderr reported "error": "". It now falls back to the exit code. - One binary revision of a path nulled its added/deleted/net, dropping real measured text churn from the ranking. The text sums survive and binary_revisions reports the exposure beside them. log.diffMerges is pinned to separate on the command line, because a user config of off makes the merge pass emit no rows at all. Git output is decoded with surrogateescape rather than replace, so two distinct non-UTF-8 paths cannot collapse into one key. evidence-gathering.md documents the changed output. 19 subprocess-level tests over real git fixture repos cover every case above; the suite goes 81 -> 100. * feat(bmm): add a pending-stories gate and align the retrospective docs `detect-epic` now also reports `pending_stories` — the selected epic's story keys that are not `done`, scoped to that epic alone — and SKILL.md branches on it before Phase 1: interactively the user is shown the list and may decline, headless the run proceeds and records the list as an assumption. An epic supplied on the invocation skips detection, so the instructions say plainly that the check did not run rather than implying it did. Both scripts are built with `add_help=False`. `-h`/`--help` previously printed argparse's usage text on stdout and exited 0, breaking the JSON-only stdout contract the scripts sell to their machine consumer; help is now an ordinary unrecognized argument routed through the existing JSON error path. SKILL.md also gained the branch for a `detect-epic` that exits non-zero, which is the normal path for a stories-mode project with no sprint-status.yaml. The references are brought back into agreement with what the scripts do after the last three batches: the close-out invocation quotes its values and pins the `--date` format that now hard-fails a non-conforming value; the result JSON documents `retro_key_found: null` and which failures carry `restored`; the retro key is stated to read `done` even for a rejected epic, so a verdict-aware gate must read the document frontmatter; Phase 3 gains an inline fallback when bmad-party-mode is absent; Phase 4 specifies the previous-retro follow-through record the close-out consumes; and the god-class view is rewritten for the current git_evidence keys. Suite goes from 100 to 111 tests. * chore(bmm): align retrospective script conventions and test harness Adopt the invocation convention the quick-dev and dev-auto skills settled on, and make the git_evidence test fixtures independent of the machine they run on. All five documented invocations become `uv run --no-cache`: the shared cache may be unwritable in a sandbox and may serve a stale build. Both scripts drop their shebangs entirely — they are only ever launched via `uv run`, so a shebang and executable bit are a false affordance; the PEP 723 blocks stay, since that is what uv reads. The encoding audit that change implies found one implicit site left: `_load_yaml` now pins the ruamel emitter's encoding, which is what encodes the bytes `_dump_bytes` writes to the user's file. `_git` and `_git_unchecked` passed an env of only the four GIT_* identity vars, so PATH and HOME were wiped and git resolved through os.defpath — the fixtures ran a different binary than the script under test, and failed outright wherever git lives only in /opt/homebrew, /usr/local or a nix store. Both now share a `_git_env` that inherits the environment, strips ambient GIT_* vars that would redirect or reconfigure the fixture, and pins identity, both gitconfig sources, gitattributes and the locale. `_run` parsed stdout bare, so a crash surfaced as JSONDecodeError with the real traceback hidden in the discarded stderr; it now routes through the `_json` guard the rest of the file already used. Both test files gain the repo's established runner footer and the PEP 723 header that makes it self-bootstrap under `uv run <file>`. No test's assertions change; the suite holds at 111 passing, and passes under a gitconfig setting commit.gpgsign, core.autocrlf, core.hooksPath and init.defaultBranch against it. * feat(bmm): aim detect-epic at a supplied epic and reject unfinished ones Unattended retros need the unfinished-story gate on the orchestrator path and a hard machine verdict when delivery is incomplete. detect-epic gains optional --epic N so -H <epic> can request the same pending_stories list auto-detect already returns for its picked epic. Non-positive values fail as JSON (exit 1). The skill now always runs detect-epic for a supplied epic with that flag, and the acceptance rubric forces rejected when pending_stories is non-empty — headless included; interactive humans may still override. Suite 111 → 115. * fix(bmm): harden retrospective script contracts after re-review - reject three-dot --range values in git_evidence.py: A...B passed the guard and silently measured a symmetric difference - move the directory fsync in _atomic_write past the failure path so a post-rename fsync error can no longer report "restored": true about a write that landed - validate --verdict against the frontmatter vocabulary and spell accepted-with-open-items one way across every machine-read surface - emit story_count from detect-epic so a typo'd epic number is distinguishable from a finished epic, and teach the SKILL gate to treat 0 as suspect - align the dev agent's ER menu description with the reworked skill * fix(bmm): reject typed retrospective statuses * docs: fix retrospective sidebar order * ci(bmm): run retrospective Python tests * fix(bmm): harden retrospective script execution Reject non-positive epic numbers before update processing. Keep explicit repositories authoritative when invoked from Git hooks. * docs(bmm): rewrite retrospective skill prose in plain language Remove invented metaphor and jargon from the skill's prompt surface (SKILL.md and references): witnesses/interrogates, testimony, seams, close-out, mining, and similar phrasing are replaced with plain equivalents. Rename Phase 5 "Close" to "Finalize" and the "Honest degradation" section to "Missing evidence", syncing cross-references. Industry-standard terms (god-class, churn) and BMAD vocabulary (declared/profiled, party mode) are kept. No operational rule, script contract, or verdict vocabulary changes. * docs(bmm): refine retrospective prose --------- Co-authored-by: Alex Verkhovsky <alexey.verkhovsky@gmail.com>