diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ed778cd..ff43560f2 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/docs/FEATURES.md b/docs/FEATURES.md index e028f7a99..a1f263f95 100644 --- a/docs/FEATURES.md +++ b/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. diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index d5029e117..839f2fc41 100644 --- a/src/bmad_loop/checks.py +++ b/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", diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 7e9658d9a..a5cec190d 100644 --- a/src/bmad_loop/cli.py +++ b/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: diff --git a/src/bmad_loop/plugins/loader.py b/src/bmad_loop/plugins/loader.py index bee36dbe4..e2adc7879 100644 --- a/src/bmad_loop/plugins/loader.py +++ b/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]: diff --git a/tests/test_cli.py b/tests/test_cli.py index 133e96828..e5422c0d5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -10621,6 +10621,203 @@ def test_validate_without_a_json_attribute_still_renders_text(project, capsys): assert not out.lstrip().startswith("{") +# ----------------------- #765: plugin manifests ------------------------------ + + +def _validate_with_plugin(project, monkeypatch, capsys, name, manifest, *, files=None, policy=None): + """A passing project plus one committed project-local plugin, so any verdict + change is the plugin's alone. Committed because an untracked plugin dir would + fail `git.worktree-clean` and make every rc assertion here meaningless.""" + _make_validate_pass(project, monkeypatch, capsys, **({"policy": policy} if policy else {})) + pdir = project.project / ".bmad-loop" / "plugins" / name + pdir.mkdir(parents=True) + (pdir / "plugin.toml").write_text(manifest) + for rel, text in (files or {}).items(): + (pdir / rel).write_text(text) + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", f"plugin {name}") + return pdir / "plugin.toml" + + +def _plugin_findings(doc): + return [f for f in doc["findings"] if f["check"] == "plugins.manifests"] + + +def test_plugin_manifest_check_is_registered(): + from bmad_loop.checks import VALIDATE_CHECKS + + assert "plugins.manifests" in VALIDATE_CHECKS + + +def test_validate_fails_a_malformed_toml_plugin_manifest(project, capsys, monkeypatch): + """#765: before this, the first reader of a broken project `plugin.toml` was + `PluginRegistry.build` in `Engine.__init__`, after the run was published. The + pass fixture makes the plugin the ONLY problem, so rc 1 is its verdict alone.""" + toml = _validate_with_plugin(project, monkeypatch, capsys, "broken", "[plugin]\nname = \n") + + rc = cli.main(["validate", "--project", str(project.project)]) + out, err = capsys.readouterr() + assert rc == 1 + assert f"FAIL: plugin {toml}: invalid TOML" in err # names the manifest path + assert "plugin manifests ok" not in out.lower() + + +@pytest.mark.parametrize( + ("body", "match"), + [ + ("[other]\nx = 1\n", "missing [plugin] table"), + ('[plugin]\nname = "bad"\napi_version = "x"\n', "api_version must be an integer"), + ], + ids=["missing-plugin-table", "invalid-field"], +) +def test_validate_json_reports_an_invalid_plugin_manifest( + project, capsys, monkeypatch, body, match +): + """The `--json` leg: one whole document at rc 1 with the FAIL inside it, and + nothing on stderr (`machine_json` parses ALL of stdout and asserts stderr empty).""" + toml = _validate_with_plugin(project, monkeypatch, capsys, "bad", body) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys, rc=1) + assert doc["ok"] is False + assert doc["counts"]["problem"] == 1 + [finding] = _plugin_findings(doc) + assert finding["severity"] == "problem" + assert str(toml) in finding["message"] and match in finding["message"] + # A failed manifest check must not cost the gates after it their findings. + checks = {f["check"] for f in doc["findings"]} + assert {"git.worktree-clean", "hooks.registered", "skills.base"} <= checks + + +def test_validate_passes_a_valid_project_plugin(project, capsys, monkeypatch): + _validate_with_plugin( + project, monkeypatch, capsys, "fine", '[plugin]\nname = "fine"\napi_version = 1\n' + ) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys) + [finding] = _plugin_findings(doc) + assert finding["severity"] == "ok" + assert "fine" in finding["detail"]["plugins"] + + +def test_validate_warns_on_an_api_mismatched_plugin_without_leaking_the_warning( + project, capsys, monkeypatch +): + """`load_plugins` skips a third-party manifest on an unsupported api_version via + `warnings.warn`. Validate reports it as a warning finding instead — rc stays 0 — + and the Python warning itself must not reach stderr, in either output mode.""" + _validate_with_plugin( + project, monkeypatch, capsys, "future", '[plugin]\nname = "future"\napi_version = 999\n' + ) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys) + warned = [f for f in _plugin_findings(doc) if f["severity"] == "warning"] + assert len(warned) == 1 + assert "'future'" in warned[0]["message"] and "api_version 999" in warned[0]["message"] + assert ( + "future" + not in next(f for f in _plugin_findings(doc) if f["severity"] == "ok")["detail"]["plugins"] + ) + + assert cli.main(["validate", "--project", str(project.project)]) == 0 + out, err = capsys.readouterr() + assert err == "" + assert "warning: plugin 'future' declares api_version 999" in out + + +def test_validate_never_imports_a_plugin_python_module(project, capsys, monkeypatch, tmp_path): + """Validate is the command a user runs to decide whether a checkout is safe to + run, so it reads manifests only. The plugin is even allowlisted in `[plugins] + enabled`, the one state in which `PluginRegistry.build` WOULD exec it — so a + regression to building the registry here writes the marker. + + The marker is the observable, and it is checked before the verdict: an import + also drops `__pycache__` into the plugin dir, so the rc would redden too, but + on `git.worktree-clean` — a symptom, not the cause. The `sys.modules` check + is a backstop only: the registry execs through `module_from_spec` without + registering the module there, so that line alone cannot see a regression.""" + marker = tmp_path / "IMPORTED" + module = ( + f"from pathlib import Path\nPath({str(marker)!r}).write_text('yes')\n" + "raise RuntimeError('imported')\n" + ) + _validate_with_plugin( + project, + monkeypatch, + capsys, + "evil", + '[plugin]\nname = "evil"\napi_version = 1\n[python]\nmodule = "hooks.py"\nclass = "P"\n', + files={"hooks.py": module}, + policy=CLAUDE_ONLY_POLICY + '[plugins]\nenabled = ["evil"]\n', + ) + + rc = cli.main(["validate", "--project", str(project.project), "--json"]) + out, err = capsys.readouterr() + assert not marker.exists() + assert "bmad_loop_plugin_evil" not in sys.modules and "hooks" not in sys.modules + assert (rc, err) == (0, "") + [finding] = _plugin_findings(json.loads(out)) + assert "evil" in finding["detail"]["plugins"] + + +def test_validate_json_reports_an_unlistable_plugins_dir(project, capsys, monkeypatch): + """A plugins dir that cannot be enumerated is a `plugins.manifests` problem + inside the one `--json` document, not an escaped OSError that empties stdout + and prints prose to stderr (`machine_json` asserts both).""" + _validate_with_plugin( + project, monkeypatch, capsys, "fine", '[plugin]\nname = "fine"\napi_version = 1\n' + ) + user_dir = project.project / ".bmad-loop" / "plugins" + real_iterdir = Path.iterdir + + def iterdir(self): + if self == user_dir: + raise PermissionError(13, "Permission denied", str(self)) + return real_iterdir(self) + + monkeypatch.setattr(Path, "iterdir", iterdir) + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys, rc=1) + [finding] = _plugin_findings(doc) + assert finding["severity"] == "problem" + assert str(user_dir) in finding["message"] and "unreadable" in finding["message"] + + +def test_validate_reads_plugin_manifests_from_the_configured_repo_root( + project, capsys, monkeypatch, tmp_path +): + """The engine builds its registry from `paths.repo_root`, and a `repo_root:` + override under isolation = "none" points that at another checkout. Validate + must parse the manifests that tree holds, not the project dir's: a broken one + in the code root fails here, and a broken one only the project holds is not + what the run loads. Ablation: pass `project` back to `_validate_plugin_manifests` + and both legs flip.""" + _validate_with_plugin( + project, + monkeypatch, + capsys, + "ignored", + "[plugin]\nname = \n", + policy=CLAUDE_ONLY_POLICY + '[scm]\nisolation = "none"\n', + ) + code_root = tmp_path / "code" + pdir = code_root / ".bmad-loop" / "plugins" / "loaded" + pdir.mkdir(parents=True) + toml = pdir / "plugin.toml" + toml.write_text('[plugin]\nname = "loaded"\napi_version = 1\n') + _configure_repo_root(project, code_root) + argv = ["validate", "--project", str(project.project), "--json"] + + cli.main(argv) # rc is not this check's: the config.yaml edit dirties the tree + [finding] = _plugin_findings(json.loads(capsys.readouterr().out)) + assert finding["severity"] == "ok", finding + assert "loaded" in finding["detail"]["plugins"] + + toml.write_text("[plugin]\nname = \n") + cli.main(argv) + [finding] = _plugin_findings(json.loads(capsys.readouterr().out)) + assert finding["severity"] == "problem" + assert str(toml) in finding["message"] + + def test_validation_report_renders_each_severity_verbatim(capsys): """The exact bytes of all three severities. diff --git a/tests/test_plugin_loader.py b/tests/test_plugin_loader.py index 4093c4209..15462c1f5 100644 --- a/tests/test_plugin_loader.py +++ b/tests/test_plugin_loader.py @@ -233,6 +233,23 @@ def test_invalid_toml_rejected(tmp_path): load_plugins(tmp_path) +def test_load_plugins_never_imports_a_python_module(tmp_path): + """Manifest discovery reads `[python]` as data and nothing more — the property + `validate`'s plugin-manifest check (#765) rests on. Import happens only in + `PluginRegistry.build`, behind the trust gate.""" + marker = tmp_path / "IMPORTED" + write_plugin( + tmp_path, + "evil", + '[plugin]\nname = "evil"\napi_version = 1\n[python]\nmodule = "hooks.py"\nclass = "P"\n', + files={"hooks.py": f"from pathlib import Path\nPath({str(marker)!r}).write_text('yes')\n"}, + ) + + plugins = load_plugins(tmp_path) + assert plugins["evil"].python is not None + assert not marker.exists() + + # The one substring every #480 refusal shares, across all seven guarded config # sites — a single matcher for the whole family. _WIN32_ALIAS_MATCH = "must not name a Windows device or end a component in a period or space" @@ -369,6 +386,62 @@ def test_unreadable_builtin_plugin_manifest_raises_plugin_error(monkeypatch): assert f"{names[0]}/{PLUGIN_FILE}" in str(excinfo.value) +def _fault_path_method(monkeypatch, method: str, target: Path) -> None: + """Make ``Path.`` raise EACCES for ``target`` alone. Targeted because + the packaged built-ins are a real `Path` in a source install: a blanket patch + would fire in the builtin loop and redden with the project site untouched.""" + real = getattr(Path, method) + + def faulted(self, *a, **kw): + if self == target: + raise PermissionError(13, "Permission denied", str(self)) + return real(self, *a, **kw) + + monkeypatch.setattr(Path, method, faulted) + + +@pytest.mark.parametrize( + ("method", "rel"), + [ + ("iterdir", USER_PLUGINS_REL), + # On the 3.11 floor is_dir/is_file swallow only absence errnos: EACCES on + # the stat escapes, e.g. under an unsearchable `.bmad-loop`. + ("is_dir", USER_PLUGINS_REL), + ("is_file", USER_PLUGINS_REL / "proj" / PLUGIN_FILE), + ], + ids=["list-root", "probe-root", "probe-manifest"], +) +def test_unreadable_project_plugin_discovery_raises_plugin_error( + tmp_path, monkeypatch, method, rel +): + """Every filesystem probe discovery makes, not only the manifest read, is + converted: a bare OSError escapes validate's `plugins.manifests` boundary (and + `PluginRegistry.build`'s callers), which all key on PluginError. + + ABLATION: drop the try in `_plugin_dirs` and every row raises PermissionError.""" + write_plugin(tmp_path, "proj", MINIMAL.format(name="proj")) + assert load_plugins(tmp_path)["proj"].source == "project" # healthy first + target = tmp_path / rel + _fault_path_method(monkeypatch, method, target) + with pytest.raises(PluginError, match="unreadable") as excinfo: + load_plugins(tmp_path) + assert str(tmp_path / USER_PLUGINS_REL) in str(excinfo.value) + + +def test_unlistable_builtin_plugins_dir_raises_plugin_error(monkeypatch): + """The packaged side of the same conversion: a corrupt or unreadable install + is a packaging bug, and the loader owes its callers the typed error + (`test_unreadable_builtin_plugin_manifest_raises_plugin_error` makes the same + case for the manifest read).""" + packaged = Path(str(resources.files("bmad_loop.data").joinpath("plugins"))) + # Real path, not a zip member, or the Path-level fault would be unarmed. + assert packaged.is_dir() + _fault_path_method(monkeypatch, "iterdir", packaged) + with pytest.raises(PluginError, match="unreadable") as excinfo: + load_plugins() + assert str(packaged) in str(excinfo.value) + + # ----------------------------------------------------- discovery / overlay