fix(validate): diagnose plugin manifests without importing plugin code (#765)

CHANGELOG.md

@@ -54,6 +54,9 @@ breaking changes may land in a minor release.
 - Refuse `init` when `.bmad-loop`, its `policy.toml` or `.gitignore` resolves outside the
   project, before any setup write (#771).
 
+- Parse plugin manifests in `validate` (`plugins.manifests`) without importing
+  plugin code; a malformed `plugin.toml` fails validate instead of engine start (#765).
+
 - Replace stale installed relay hooks when a project moves between Windows and POSIX.
 
 - Report stale or unverifiable Codex hook trust in `validate` and `probe-adapter`

docs/FEATURES.md

@@ -753,7 +753,7 @@ verdict unverifiable rather than certifying a different launch configuration.
 ### 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/`), an absolute hook registration for the installed 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 (including its version — a host below the **2.34** support floor gets a `git.version` **problem** and exit 1, so validate's verdict cannot disagree with run/sweep/resume's outright refusal), the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary (**probed, not just resolved**: a name that is on `PATH` but fails `--version`, typically a dead WSL/npm shim, adds an `adapter.binary-unrunnable` finding at warning severity — `adapter.binary` itself still reports ok, and validate's exit code is unchanged — [#294](https://github.com/bmad-code-org/bmad-loop/issues/294); only **packaged** profiles are probed — a project overlay's binary is resolved but never launched, so a clone cannot choose which binary this diagnostic launches; resolution still goes through your `PATH`, so what a probed name resolves to is whatever the session launch would itself run), hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — 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`.
+- `bmad-loop validate` preflights every prerequisite: BMAD config, plugin manifests (`plugins.manifests`: every bundled and project-local `plugin.toml` — project-local read from the configured `repo_root`, the tree a run loads them from — is parsed exactly as a run will parse it, and never imported — a malformed one is a **problem** naming the manifest, so it fails here instead of at engine start after the run is published; a third-party manifest on an unsupported `api_version` is a warning, since a run skips it — [#765](https://github.com/bmad-code-org/bmad-loop/issues/765)), sprint-status, git (including its version — a host below the **2.34** support floor gets a `git.version` **problem** and exit 1, so validate's verdict cannot disagree with run/sweep/resume's outright refusal), the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary (**probed, not just resolved**: a name that is on `PATH` but fails `--version`, typically a dead WSL/npm shim, adds an `adapter.binary-unrunnable` finding at warning severity — `adapter.binary` itself still reports ok, and validate's exit code is unchanged — [#294](https://github.com/bmad-code-org/bmad-loop/issues/294); only **packaged** profiles are probed — a project overlay's binary is resolved but never launched, so a clone cannot choose which binary this diagnostic launches; resolution still goes through your `PATH`, so what a probed name resolves to is whatever the session launch would itself run), hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — 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`.
 - **Where the artifact paths come from** ([#769](https://github.com/bmad-code-org/bmad-loop/issues/769), [#154](https://github.com/bmad-code-org/bmad-loop/issues/154)): `implementation_artifacts`, `planning_artifacts`, `output_folder` and `repo_root` are read from BMAD's central TOML — `_bmad/config.toml`, `_bmad/config.user.toml`, `_bmad/custom/config.toml`, `_bmad/custom/config.user.toml`, each overriding the one before, merged as BMAD's renderer merges them — and from the legacy `_bmad/bmm/config.yaml`. Each key is looked up the way the renderer resolves a short config key: a key found under more than one table (e.g. both `[core]` and `[modules.bmm]`) is refused as ambiguous, naming every location. A TOML value wins over the YAML for every key; the YAML only fills a key the TOML lacks, and with no TOML layer the YAML is read as before. A malformed or non-UTF-8 layer, a blank or non-string value, or an ambiguous key is a `bmad-config` failure, never a silent fallback to the YAML. The two artifact dirs are required; `output_folder` defaults to `{project-root}/_bmad-output` and `repo_root` to the project dir.
 - The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is normally explained by another finding (the historical unregistered-tmux fallback is the one silent exception; see the `--json` contract note in `documents.py`). On top of that, `host.win32-on-wsl-path` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`).
 - Non-invasive: drives the upstream dev primitive 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.

src/bmad_loop/checks.py

@@ -77,6 +77,7 @@
         "host.process",
         "host.win32-on-wsl-path",
         "notify.desktop-unavailable",
+        "plugins.manifests",
         "skills.base",
         "skills.base-missing",
         "skills.base-incomplete",

src/bmad_loop/cli.py

@@ -490,6 +490,12 @@ def cmd_validate(args: argparse.Namespace) -> int:
                 {"repo_root": str(paths.repo_root), "project": str(paths.project)},
             )
 
+    # The engine builds its registry from `paths.repo_root` (a `repo_root:` override
+    # under isolation = "none" points it at another checkout), so read the manifests
+    # that run will load. A failed BMAD config already failed above; fall back to
+    # the project dir so the manifest check still reports something.
+    _validate_plugin_manifests(paths.repo_root if paths is not None else project, report)
+
     # Built exactly the way run/sweep's real preflight builds it, so validate's
     # verdict and their abort cannot disagree. Deliberately NOT `[p.skill_tree for p
     # in profiles]`: that carries triage's tree, and every skills check below asks a
@@ -1517,6 +1523,48 @@ def _spec_closes_deferred(path: Path) -> tuple[tuple[str, ...], str | None]:
     return deferredwork.parse_declaration(raw)
 
 
+def _validate_plugin_manifests(root: Path, report: ValidationReport) -> None:
+    """Parse every discovered plugin manifest the way a run will (#765).
+
+    `root` is the code root the engine hands `PluginRegistry.build` —
+    `paths.repo_root`, not necessarily the project dir.
+
+    Without this the first reader of a malformed project `plugin.toml` was
+    `PluginRegistry.build` inside `Engine.__init__` — after the run's directory,
+    state and journal were already published. `load_plugins` is manifest-only
+    discovery: it never imports a `[python]` module, which matters here because
+    validate is the command a user runs to decide whether a checkout is safe to
+    run at all. `PluginRegistry.build` would exec every allowlisted module.
+
+    A PluginError is the whole message: every manifest fault names its source
+    (the manifest path, for a project plugin). `load_plugins` stops at the first
+    bad manifest, so one fault is reported per pass. A third-party manifest on an
+    unsupported api_version is skipped with `warnings.warn`, which a run keeps;
+    here it is captured and reported as a warning finding instead, so it neither
+    leaks to stderr nor breaks the `--json` stream contract.
+    """
+    import warnings
+
+    from .plugins import PluginError, load_plugins
+
+    with warnings.catch_warnings(record=True) as skipped:
+        warnings.simplefilter("always")  # the once-per-location default would drop a repeat
+        try:
+            manifests = load_plugins(root)
+        except PluginError as e:
+            manifests = None
+            report.fail("plugins.manifests", str(e))
+    for w in skipped:
+        report.warn("plugins.manifests", f"{w.message} — skipped; a run will not load it")
+    if manifests is not None:
+        names = sorted(manifests)
+        report.ok(
+            "plugins.manifests",
+            f"plugin manifests OK: {len(names)} loaded ({', '.join(names) or 'none'})",
+            {"plugins": names},
+        )
+
+
 def _validate_operator_registry(
     project: Path, paths: bmadconfig.ProjectPaths, report: ValidationReport
 ) -> None:

src/bmad_loop/plugins/loader.py

@@ -21,8 +21,9 @@
 
 from __future__ import annotations
 
+import os
 import warnings
-from collections.abc import Iterator
+from collections.abc import Callable, Iterator
 from importlib import resources
 from importlib.resources.abc import Traversable
 from pathlib import Path
@@ -65,20 +66,44 @@ def _read_manifest_text(toml: Traversable | Path, source: str) -> str:
         raise PluginError(f"plugin {source}: unreadable: {e}") from e
 
 
+def _plugin_dirs(
+    root: Traversable, sort_key: Callable[[Traversable], str]
+) -> list[tuple[Traversable, Traversable]]:
+    """Every ``(plugin dir, plugin.toml)`` pair under a plugins root, or ``[]``
+    when the root is absent — with any filesystem fault converted to PluginError.
+
+    The whole enumeration sits behind one conversion, not just the listing: on the
+    3.11 floor `Path.is_dir()`/`is_file()` swallow only absence errnos (ENOENT,
+    ENOTDIR, EBADF, ELOOP) and RAISE on EACCES, so a root whose parent is not
+    searchable faults on the existence probe, and an unsearchable entry on its
+    `is_file()`. Every consumer keys on PluginError (validate --json reports it as
+    a finding; `PluginRegistry.build` and the TUI settings screen degrade on it),
+    and the manifest read itself is already converted by `_read_manifest_text`.
+    Collected eagerly so no fault can surface mid-yield from a later probe.
+    """
+    try:
+        if not root.is_dir():
+            return []
+        found: list[tuple[Traversable, Traversable]] = []
+        for entry in sorted(root.iterdir(), key=sort_key):
+            toml = entry.joinpath(PLUGIN_FILE)
+            if entry.is_dir() and toml.is_file():
+                found.append((entry, toml))
+        return found
+    except OSError as e:
+        raise PluginError(f"plugin dir {root}: unreadable: {e}") from e
+
+
 def _discover_builtin() -> Iterator[PluginManifest]:
     packaged = resources.files("bmad_loop.data").joinpath("plugins")
-    if not packaged.is_dir():
-        return
-    for entry in sorted(packaged.iterdir(), key=lambda e: e.name):
-        toml = entry.joinpath(PLUGIN_FILE)
-        if entry.is_dir() and toml.is_file():
-            source = f"{entry.name}/{PLUGIN_FILE}"
-            yield load_manifest(
-                _read_manifest_text(toml, source),
-                source,
-                str(entry),
-                origin="builtin",
-            )
+    for entry, toml in _plugin_dirs(packaged, lambda e: e.name):
+        source = f"{entry.name}/{PLUGIN_FILE}"
+        yield load_manifest(
+            _read_manifest_text(toml, source),
+            source,
+            str(entry),
+            origin="builtin",
+        )
 
 
 def _discover_entry_points() -> Iterator[PluginManifest]:
@@ -93,15 +118,12 @@ def _discover_entry_points() -> Iterator[PluginManifest]:
 
 
 def _discover_project(project: Path) -> Iterator[PluginManifest]:
-    user_dir = project / USER_PLUGINS_REL
-    if not user_dir.is_dir():
-        return
-    for entry in sorted(user_dir.iterdir()):
-        toml = entry / PLUGIN_FILE
-        if entry.is_dir() and toml.is_file():
-            yield load_manifest(
-                _read_manifest_text(toml, str(toml)), str(toml), str(entry), origin="project"
-            )
+    # normcase: the order `sorted(Path.iterdir())` gave siblings — case-folded on
+    # Windows — so which of two same-named manifests wins the overlay is unchanged.
+    for entry, toml in _plugin_dirs(project / USER_PLUGINS_REL, lambda e: os.path.normcase(e.name)):
+        yield load_manifest(
+            _read_manifest_text(toml, str(toml)), str(toml), str(entry), origin="project"
+        )
 
 
 def discover(project: Path | None = None) -> Iterator[PluginManifest]: