fix(validate): accept merged bmad-review, drop unreleased verification-gap requirement (#260)

CHANGELOG.md

@@ -67,6 +67,41 @@ breaking changes may land in a minor release.
 
 ### Fixed
 
+- **`validate` requires the review skills your `bmad-dev-auto` actually invokes (#260).** The
+  preflight held every project to a fixed catalog that included `bmad-review-verification-gap`,
+  which no tagged BMAD-METHOD release ships (absent from v6.10.0; on current sources only a
+  forwarder to the merged `bmad-review`), and misdiagnosed it as "install the BMad Method (bmm)
+  module" on projects where bmm _was_ installed — so `validate`/`run`/`resume`/`sweep` could not
+  pass on a stock install. The required reviewers are now read from the installed skill itself:
+  its `customize.toml` `[[workflow.review_layers]]` when present (honoring disabled layers and
+  `_bmad/custom/bmad-dev-auto.toml` overrides), else the reviewers `step-04-review.md` names
+  inline. A merged-`bmad-review` install needs only `bmad-review`; a v6.10.0 install needs the
+  two hunters it names; and a tree whose configured layers reference a skill it does not have is
+  now caught by a new `skills.review-layer-missing` problem instead of passing and then failing
+  on every dev run. Unreadable configs fall back to the previous static requirement, and all
+  messages name the real remedy (install/update bmm, BMAD-METHOD >= 6.10.0).
+
+  Derivation is held to what the run really resolves:
+  - Override merging matches BMAD's own resolver (`resolve_customization.py`) exactly — arrays of
+    tables opt into keyed merge only when _every_ combined item carries the same identifier, and
+    `code` is checked before `id`; anything mixed or key-less appends. Keying on `id` alone
+    silently dropped base layers in one direction and kept superseded ones in the other.
+  - An override that is not valid TOML no longer abandons derivation: the resolver skips a broken
+    layer and carries on, so the preflight does too, reporting the file as a
+    `skills.customize-unreadable` warning.
+  - A `customize.toml` whose `workflow` key is valid TOML of the wrong _shape_ (a string, list or
+    number) no longer raises `AttributeError` out of `validate`/`run`/`resume`/`sweep`.
+  - Disabling every review layer is reported as `skills.review-layers-empty` rather than passing:
+    `step-04-review.md` HALTs blocked with `no active review layers`.
+  - Layers that cannot be resolved statically — gated by a `when` condition the model evaluates at
+    run time, or naming a skill in a phrasing this check cannot confirm is a handoff — are reported
+    as `skills.review-layer-unresolved` **warnings** and never block. `validate` and the run
+    preflight now branch on severity, so an advisory can no longer abort a run.
+  - Isolated worktrees get what was validated: `provision_worktree` copies the review skills the
+    project's own layers name (not just the fixed base catalog) and seeds `_bmad/custom/`, whose
+    `*.user.toml` layer the upstream installer gitignores — without it the preflight resolved one
+    layer set from the main checkout while the worktree run resolved another.
+
 - **`notify.desktop` works on macOS/Windows, and warns when it can't (#231).** The desktop
   notification channel was `notify-send`-only, so on macOS and Windows `notify.desktop` (default
   `true`) was silently inert — every "a human is needed" path (escalations, deferrals,

README.md

@@ -40,7 +40,7 @@ Inspired by the original [bmad-automator](https://github.com/bmad-code-org/bmad-
 
 - **Python 3.11+**, a **terminal multiplexer** (tmux is the bundled default), and a supported coding CLI — `claude` by default; `codex`, `gemini`, `copilot`, and `antigravity` (`agy`) via [profiles](#other-coding-clis).
 - **Linux or macOS** (or **Windows via WSL**, which _is_ Linux — it runs as-is). tmux is the bundled terminal-multiplexer backend (externals like the [herdr adapter](https://github.com/pbean/bmad-loop-adapter-herdr) co-install as packages and self-register — see [Terminal multiplexer backends](docs/multiplexer-backends.md)), and all of it sits behind a pluggable **registry** of OS seams (transport, process lifecycle, hook interpreter) with availability-aware selection — env var → persisted `[mux] backend` choice (`bmad-loop mux set <name>`) → platform default (`psmux` on Windows, `tmux` elsewhere) → first available platform match — so a native-Windows backend slots in as new files + a registration line each, with no engine edits — see [Porting bmad-loop to a new OS](docs/porting-to-a-new-os.md). Native Windows is not yet shipped.
-- A **BMAD v6 project** (`_bmad/bmm/config.yaml`, a `sprint-status.yaml` from `bmad-sprint-planning`) with the upstream `bmad-dev-auto` skill (and the three review-hunter skills its step-04 invokes inline: `bmad-review-adversarial-general`, `bmad-review-edge-case-hunter`, `bmad-review-verification-gap`) and the bmad-loop skill module from this repo installed (`bmad-loop-resolve`, `bmad-loop-sweep` — see [Installing the skill module](#installing-the-skill-module)). Standard BMAD skills stay untouched.
+- A **BMAD v6 project** (`_bmad/bmm/config.yaml`, a `sprint-status.yaml` from `bmad-sprint-planning`) on **BMAD-METHOD ≥ 6.10.0**, with the upstream `bmad-dev-auto` skill (and the review-layer skills its step-04 invokes inline: `bmad-review-adversarial-general` + `bmad-review-edge-case-hunter`, or the merged `bmad-review` skill that supersedes them in newer releases) and the bmad-loop skill module from this repo installed (`bmad-loop-resolve`, `bmad-loop-sweep` — see [Installing the skill module](#installing-the-skill-module)). Standard BMAD skills stay untouched.
 
 ## Quick start
 
@@ -243,19 +243,19 @@ Bundle dev sessions can themselves append new deferred entries (split-off goals,
 
 ## Installing the skill module
 
-The orchestrator drives the upstream `bmad-dev-auto` skill as its inner dev primitive — unmodified, so there is no fork to keep in sync; it both implements and (re-invoked on the done spec) runs the follow-up review — plus its own bundled `bmad-loop-*` skills for escalation, sweep, and setup. Your standard BMAD install is never modified. The three bundled skills ship in the `bmad-loop` wheel (canonical source: `src/bmad_loop/data/skills/`, BMAD module code `bmad-loop`) so `bmad-loop init` lays them down for you; `bmad-dev-auto` and the three review-hunter skills its step-04 invokes inline are prerequisites installed by the BMad Method (bmm) module:
+The orchestrator drives the upstream `bmad-dev-auto` skill as its inner dev primitive — unmodified, so there is no fork to keep in sync; it both implements and (re-invoked on the done spec) runs the follow-up review — plus its own bundled `bmad-loop-*` skills for escalation, sweep, and setup. Your standard BMAD install is never modified. The three bundled skills ship in the `bmad-loop` wheel (canonical source: `src/bmad_loop/data/skills/`, BMAD module code `bmad-loop`) so `bmad-loop init` lays them down for you; `bmad-dev-auto` and the review-layer skills its step-04 invokes inline are prerequisites installed by the BMad Method (bmm) module:
 
-| Skill                             | Role                                                                                            |
-| --------------------------------- | ----------------------------------------------------------------------------------------------- |
-| `bmad-dev-auto`                   | unattended implementation + follow-up review (**upstream** — bmm prerequisite, not bundled)     |
-| `bmad-review-adversarial-general` | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled)                      |
-| `bmad-review-edge-case-hunter`    | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled)                      |
-| `bmad-review-verification-gap`    | inline step-04 review layer, newest (BMAD-METHOD#2550) (**upstream** — bmm prereq, not bundled) |
-| `bmad-loop-resolve`               | interactive CRITICAL-escalation resolution (`/bmad-loop-resolve <story>`)                       |
-| `bmad-loop-sweep`                 | deferred-work ledger triage (automation-only)                                                   |
-| `bmad-loop-setup`                 | registers the module in `_bmad/` config + help                                                  |
+| Skill                             | Role                                                                                        |
+| --------------------------------- | ------------------------------------------------------------------------------------------- |
+| `bmad-dev-auto`                   | unattended implementation + follow-up review (**upstream** — bmm prerequisite, not bundled) |
+| `bmad-review-adversarial-general` | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled)                  |
+| `bmad-review-edge-case-hunter`    | inline step-04 review layer (**upstream** — bmm prerequisite, not bundled)                  |
+| `bmad-review`                     | merged lens-based reviewer, supersedes the hunters (**upstream** — bmm prereq, not bundled) |
+| `bmad-loop-resolve`               | interactive CRITICAL-escalation resolution (`/bmad-loop-resolve <story>`)                   |
+| `bmad-loop-sweep`                 | deferred-work ledger triage (automation-only)                                               |
+| `bmad-loop-setup`                 | registers the module in `_bmad/` config + help                                              |
 
-`bmad-loop validate` preflights all four upstream skills; a target project on a pre-July bmm install missing `bmad-review-verification-gap` (or a `bmad-dev-auto` without its `customize.toml` review-layer config) is reported with bmm-module remediation before any run starts.
+`bmad-loop validate` preflights `bmad-dev-auto` (always) plus the review skills that copy of the skill will actually invoke — read from its `customize.toml` review layers, or from `step-04-review.md` on releases that name their reviewers inline. So a merged-`bmad-review` install needs only `bmad-review`, a v6.10.0 install needs the two hunters it names, and a tree whose configured layers reference a skill it does not have is reported instead of failing on every dev run. Missing skills (or a `bmad-dev-auto` without its `customize.toml`) are reported with bmm-module remediation before any run starts.
 
 **Via uv + `bmad-loop init` (self-sufficient).** Installing the tool and running `init` is all you need — `init` installs the `bmad-loop-*` skills into `.claude/skills/` (claude) and/or `.agents/skills/` (codex/gemini) for the CLIs you select, alongside the hooks and policy:
 

docs/FEATURES.md

@@ -175,7 +175,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se
 ### Setup & install
 
 - `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`.
-- `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the three inline review-hunter skills `bmad-dev-auto` invokes (`bmad-review-adversarial-general`, `bmad-review-edge-case-hunter`, `bmad-review-verification-gap`) plus its `customize.toml`.
+- `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the review skills the installed `bmad-dev-auto` actually invokes — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`.
 - Non-invasive: drives the upstream `bmad-dev-auto` skill unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified.
 
 ### Command reference

docs/setup-guide.md

@@ -97,8 +97,59 @@ installs all the bundled skills together (`bmad-loop-resolve`, `bmad-loop-sweep`
 the orchestrator normalizes the ledger to. The dev primitive `bmad-dev-auto` is
 **not** bundled: it is the upstream skill the orchestrator drives (for both
 implementation and the follow-up review), installed by the BMad Method (bmm)
-module. `bmad-loop validate` checks it — plus the three review hunters it
-invokes inline — are present before a run starts.
+module. `bmad-loop validate` checks it — plus the review layers it invokes
+inline — are present before a run starts.
+
+**Minimum BMAD-METHOD: v6.10.0** for sprint mode. Beyond `bmad-dev-auto` itself (with
+its `customize.toml`), `bmad-loop validate` holds you to no fixed list of review
+skills: it reads the `bmad-dev-auto` you actually have installed and requires the
+reviewers that copy will actually invoke.
+
+- **Layer-driven (post-6.10.0)** — `customize.toml` defines
+  `[[workflow.review_layers]]`, and each layer's `instruction` names the skill it hands
+  off to. Current sources define four layers: `blind-hunter`, `edge-case-hunter`, and
+  `verification-gap`, each invoking the merged `bmad-review` skill with one lens, plus
+  `intent-alignment`, a self-contained prompt that invokes no skill at all. Such a tree
+  needs `bmad-review` and none of the standalone hunters. An intermediate release whose
+  layers name the standalone hunters needs exactly those instead.
+- **v6.10.0** — no `review_layers` section at all; `step-04-review.md` names
+  `bmad-review-adversarial-general` and `bmad-review-edge-case-hunter` inline, so those
+  two are what is required.
+
+A layer disabled with an empty `instruction`, or replaced in
+`_bmad/custom/bmad-dev-auto.toml` by a recipe that runs something else, requires
+nothing. If the shape cannot be read at all, `validate` falls back to requiring the two
+v6.10.0 hunters — which a tree carrying `bmad-review` satisfies.
+
+`bmad-review-verification-gap` is no longer required unconditionally (#260): no tagged
+release ships it as a standalone skill, and on current sources it is only a thin
+forwarder to `bmad-review`. It is required exactly when your installed layers name it.
+
+### Customizing the review layers
+
+Your project overrides in `_bmad/custom/bmad-dev-auto.toml` (team) and
+`_bmad/custom/bmad-dev-auto.user.toml` (personal, gitignored by the bmm installer) are
+applied before the requirement is computed, using the same merge rules BMAD's own
+resolver uses: an array of tables merges by key only when _every_ entry — default and
+override alike — carries the same `code` or `id`; otherwise the override appends. So
+adding a layer with a new `id` adds a requirement, and replacing one by `id` moves it.
+
+Two things `validate` reports but never blocks on, because neither can be decided
+without running the review:
+
+- a layer gated by a `when` condition, which the model evaluates against the diff;
+- a layer naming a skill in a phrasing that is not recognizably a handoff (the
+  convention is ``Invoke the `skill-name` skill``).
+
+Both come back as `skills.review-layer-unresolved` warnings, so a customized layer can
+never block a run the way the old fixed catalog did. What _is_ a problem: every layer
+disabled at once (`skills.review-layers-empty`), which makes `bmad-dev-auto` HALT
+blocked with `no active review layers`.
+
+Under `isolation = "worktree"` the skills your layers name are copied into each unit's
+worktree along with `_bmad/custom/`, so an isolated run resolves the same layers
+`validate` checked — including from the gitignored `*.user.toml`, which a fresh checkout
+would not carry.
 
 ## Choosing which CLIs to drive
 

src/bmad_loop/checks.py

@@ -68,6 +68,10 @@
         "skills.base",
         "skills.base-missing",
         "skills.base-incomplete",
+        "skills.review-layer-missing",
+        "skills.review-layer-unresolved",
+        "skills.review-layers-empty",
+        "skills.customize-unreadable",
         "skills.stories-dispatch",
         "skills.stories-dispatch-missing",
         "skills.stories-dispatch-stale",

src/bmad_loop/cli.py

@@ -353,14 +353,17 @@ def cmd_validate(args: argparse.Namespace) -> int:
                     {"role": role, "model": cfg.model, "profile": prof.name},
                 )
 
-    base_problems = install.missing_base_skills(project, [p.skill_tree for p in profiles])
-    if profiles and not base_problems:
+    base_findings = install.missing_base_skills(project, [p.skill_tree for p in profiles])
+    # gated on PROBLEMS, not on any finding: an advisory review layer (a `when`
+    # gate, a phrasing this check can't confirm, a broken override) is a warning
+    # that must ride alongside the ok line rather than suppress it.
+    if profiles and not any(f.severity == "problem" for f in base_findings):
         report.ok(
             "skills.base",
-            "upstream skills present (bmad-dev-auto + review hunters)",
+            "upstream skills present (bmad-dev-auto + review layers)",
             {"trees": list(dict.fromkeys(p.skill_tree for p in profiles))},
         )
-    report.extend(base_problems)
+    report.extend(base_findings)
 
     if getattr(args, "json", False):
         # getattr, not args.json: cmd_validate is called directly by tests (and by
@@ -480,11 +483,12 @@ def _mux_set(project: Path, args: argparse.Namespace) -> int:
 
 def _require_base_skills(project: Path, pol, *, require_stories: bool = False) -> bool:
     """Preflight the upstream skills the orchestrator drives (bmad-dev-auto + the
-    three review hunters it invokes inline).
+    review layers it invokes inline).
 
     Returns True when everything is in place; otherwise prints the problems and
     returns False so the caller can abort before spawning any session (a missing
     skill would otherwise stall as an `Unknown command` until the run times out).
+    Warnings are printed but never abort — only ``problem`` findings block.
 
     ``require_stories`` additionally content-probes bmad-dev-auto for folder+id
     dispatch — stories mode needs a newer skill than sprint mode, so an older
@@ -498,9 +502,16 @@ def _require_base_skills(project: Path, pol, *, require_stories: bool = False) -
             skill_trees.append(get_profile(name, project).skill_tree)
         except ProfileError:
             continue
-    problems = install.missing_base_skills(project, skill_trees)
+    findings = install.missing_base_skills(project, skill_trees)
     if require_stories:
-        problems += install.missing_stories_support(project, skill_trees)
+        findings += install.missing_stories_support(project, skill_trees)
+    # Severity decides. A review layer we can't statically resolve (a `when` gate,
+    # an unrecognized handoff phrasing, an unparseable override) is reported and
+    # then stepped over: aborting on it would be the false FAIL of #260, only now
+    # on every run rather than only on validate.
+    problems = [f for f in findings if f.severity == "problem"]
+    for warning in (f for f in findings if f.severity != "problem"):
+        print(f"warning: {warning.message}", file=sys.stderr)
     if problems:
         for problem in problems:
             print(f"FAIL: {problem.message}", file=sys.stderr)