diff --git a/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 5f6ff04f..10d4cf7f 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -14,7 +14,10 @@ bmad-loop a new CLI: - **The advanced case — a new adapter class.** If the CLI does _not_ fit that transport (e.g. an HTTP/SSE service), see [Writing a new adapter class](#writing-a-new-adapter-class) for the - `CodingCLIAdapter` ABC. + `CodingCLIAdapter` ABC — and + [Shipping a new adapter class out-of-tree](#shipping-a-new-adapter-class-out-of-tree) + to register it (and its profile) from a co-installed package with **zero core + edits**, the same way a transport backend ships out-of-tree. ## Two axes: CLI vs transport @@ -22,7 +25,11 @@ These are independent and abstracted separately: - **CLI axis** — `CodingCLIAdapter` (`adapters/base.py`): _which_ binary to launch, how the prompt is rendered, the hook dialect, where the transcript lives. The - generic adapter + a TOML profile cover this; the rest of this guide is about it. + generic adapter + a TOML profile cover the common case; a CLI needing its own + adapter class registers it via `register_adapter(...)` + (`adapters/registry.py`) — selected by the profile's `adapter` field and + shippable out-of-tree, just like a transport backend. Most of this guide is + about this axis. - **Transport axis** — `TerminalMultiplexer` (`adapters/multiplexer.py`): how sessions, windows, and panes are created, observed, and torn down. The generic adapter never shells out itself — it goes through `self.mux`, obtained from @@ -357,23 +364,24 @@ resolves to `claude`. ### `CLIProfile` -| Field | Required | Default | Meaning | -| ---------------------------------- | -------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `name` | ✅ | — | Profile id, also the `--cli` value and override key. | -| `binary` | ✅ | — | Executable to launch (resolved on `PATH`). | -| `[hooks]` | ✅ | — | The `HookSpec` table (see below). | -| `skill_tree` | | `.claude/skills` | Project-relative tree this CLI reads skills from (`.agents/skills` for codex/gemini); `bmad-loop init` installs the `bmad-loop-*` skills here. Must be relative. | -| `prompt_template` | | `{prompt}` | How the canonical `/skill args` prompt is rendered. Placeholders: `{prompt}` (whole string), `{skill}` (leading slash-command name, no `/`), `{args}` (the remainder). | -| `launch_args` | | `()` | Extra argv passed at launch, e.g. `["-i"]` to stay interactive (gemini/copilot). | -| `bypass_args` | | `()` | Flags that bypass permission/approval prompts for unattended runs (e.g. `--allow-all-tools`). | -| `model_flag` | | `--model` | Flag used to pass the model name when one is configured. | -| `env` | | `{}` | Extra environment variables for the session. | -| `usage_parser` | | `none` | Which transcript token parser to use — one of `claude-jsonl`, `codex-rollout`, `gemini-chat`, `copilot-events`, `none`. | -| `usage_grace_s` | | `0.0` | Seconds to keep polling the transcript for token totals after the session ends. `0` = read once. Raise it for CLIs that flush totals only on shutdown (copilot writes `modelMetrics` ~1s after the turn-end hook). Must be ≥ 0. | -| `stop_without_result_nudges` | | unset (use global) | Per-adapter floor for Stop-without-result nudges. Leave unset to inherit `limits.stop_without_result_nudges`. Raise it for CLIs that fire a turn-end hook _per response turn_ (copilot's `agentStop`), where the global default of 1 declares them stalled too early. Must be ≥ 0 if set. | -| `subagent_stop_without_transcript` | | `false` | Set `true` for CLIs that fire the turn-end hook for _subagent_ turns too, with an empty `transcriptPath` and a tool-use session id (copilot's `agentStop`). A `Stop` carrying no transcript is then treated as a subagent stop and ignored, so the main session's real turn-end drives completion. Leave `false` and every `Stop` is the main turn-end. | -| `first_run_note` | | `""` | Human note printed by `init` about a manual first-run/auth step this CLI needs. | -| `seed_files` | | `()` | Project-relative gitignored configs (MCP/CLI settings) a `git worktree add` checkout omits; `provision_worktree` copies them into isolated dev/review worktrees. Must be relative. | +| Field | Required | Default | Meaning | +| ---------------------------------- | -------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `name` | ✅ | — | Profile id, also the `--cli` value and override key. | +| `binary` | ✅ | — | Executable to launch (resolved on `PATH`). | +| `[hooks]` | ✅ | — | The `HookSpec` table (see below). | +| `adapter` | | `generic` | Which adapter **class** drives this CLI, resolved against the adapter registry (`adapters/registry.py`) — not a hardcoded enum. `generic` = the bundled tmux + hook-signal adapter; `opencode-http` = the bundled HTTP/SSE adapter; an out-of-tree package registers its own (see [Shipping a new adapter class out-of-tree](#shipping-a-new-adapter-class-out-of-tree)). Read but not validated at parse time — an unknown kind fails loud at construction and as a `validate` finding, both against the live registry. | +| `skill_tree` | | `.claude/skills` | Project-relative tree this CLI reads skills from (`.agents/skills` for codex/gemini); `bmad-loop init` installs the `bmad-loop-*` skills here. Must be relative. | +| `prompt_template` | | `{prompt}` | How the canonical `/skill args` prompt is rendered. Placeholders: `{prompt}` (whole string), `{skill}` (leading slash-command name, no `/`), `{args}` (the remainder). | +| `launch_args` | | `()` | Extra argv passed at launch, e.g. `["-i"]` to stay interactive (gemini/copilot). | +| `bypass_args` | | `()` | Flags that bypass permission/approval prompts for unattended runs (e.g. `--allow-all-tools`). | +| `model_flag` | | `--model` | Flag used to pass the model name when one is configured. | +| `env` | | `{}` | Extra environment variables for the session. | +| `usage_parser` | | `none` | Which transcript token parser to use — one of `claude-jsonl`, `codex-rollout`, `gemini-chat`, `copilot-events`, `none`. | +| `usage_grace_s` | | `0.0` | Seconds to keep polling the transcript for token totals after the session ends. `0` = read once. Raise it for CLIs that flush totals only on shutdown (copilot writes `modelMetrics` ~1s after the turn-end hook). Must be ≥ 0. | +| `stop_without_result_nudges` | | unset (use global) | Per-adapter floor for Stop-without-result nudges. Leave unset to inherit `limits.stop_without_result_nudges`. Raise it for CLIs that fire a turn-end hook _per response turn_ (copilot's `agentStop`), where the global default of 1 declares them stalled too early. Must be ≥ 0 if set. | +| `subagent_stop_without_transcript` | | `false` | Set `true` for CLIs that fire the turn-end hook for _subagent_ turns too, with an empty `transcriptPath` and a tool-use session id (copilot's `agentStop`). A `Stop` carrying no transcript is then treated as a subagent stop and ignored, so the main session's real turn-end drives completion. Leave `false` and every `Stop` is the main turn-end. | +| `first_run_note` | | `""` | Human note printed by `init` about a manual first-run/auth step this CLI needs. | +| `seed_files` | | `()` | Project-relative gitignored configs (MCP/CLI settings) a `git worktree add` checkout omits; `provision_worktree` copies them into isolated dev/review worktrees. Must be relative. | ### `HookSpec` (the `[hooks]` table) @@ -523,6 +531,83 @@ adapter against a scripted stdlib FakeOpencode (no binary, no network beyond smoke-checks the pinned HTTP contract against a real local binary — skipped when absent, zero tokens spent. +### Shipping a new adapter class out-of-tree + +A new adapter class does **not** have to live in the bmad-loop repo. Which class +drives a CLI is data — the profile's `adapter` field, resolved against the +adapter registry ([`adapters/registry.py`](../src/bmad_loop/adapters/registry.py)) +— so a co-installed package registers its own kind with no edit to any core +`.py`, exactly as a transport backend ships via `bmad_loop.mux_backends` +([the transport contract](#the-transport-contract-for-a-backend-author)). The +reference layout is the out-of-tree +[herdr backend](https://github.com/pbean/bmad-loop-adapter-herdr). + +Ship a pip/uv package that advertises **two entry points**: + +- **`bmad_loop.adapters`** → a module whose import registers the kind. Core imports + it (after the builtins, so a bundled name always wins) before it resolves any + adapter: + + ```python + # acme_adapter/__init__.py + from bmad_loop.adapters.registry import AdapterBuilder, register_adapter + + def _load(): # lazy: imported only when a run builds this kind, + from .acme import AcmeAdapter, AcmeDevAdapter # so an optional dep stays unpaid + return AdapterBuilder( + plain=AcmeAdapter, # the plain class + dev=AcmeDevAdapter, # the _DevSynthesisMixin-composed dev/review class + construct_error=(), # exception type(s) __init__ may raise; () = none + ) + + register_adapter("acme", needs_mux=True, load=_load) # needs_mux: does it drive a multiplexer? + ``` + + ```toml + # pyproject.toml + [project.entry-points."bmad_loop.adapters"] + acme = "acme_adapter" + ``` + +- **`bmad_loop.profiles`** → a callable returning `CLIProfile`s (or an iterable of + them), so the profile that _selects_ your kind ships with it — no project TOML + required. Precedence is packaged < entry-point < project, so a project TOML can + still override it: + + ```python + # acme_adapter/__init__.py (same package) + from bmad_loop.adapters.profile import CLIProfile, HookSpec + + def profiles(): + return [CLIProfile(name="acme", binary="acme", adapter="acme", + hooks=HookSpec("none", "", {}))] + ``` + + ```toml + [project.entry-points."bmad_loop.profiles"] + acme = "acme_adapter:profiles" + ``` + +Once co-installed, `bmad-loop adapters` lists the kind and the profiles that select +it, `bmad-loop validate` checks the reference (an `adapter.kind` finding, resolved +against the live registry — never a hardcoded set), and a run whose +`[adapter] name = "acme"` selects it. A broken package degrades to a +`bmad-loop adapters` / `validate` warning, never a selection crash. + +Two seam facts worth internalizing: + +- **`needs_mux`** gates whether the run bootstrap resolves and usability-checks the + shared terminal multiplexer for your family. A tmux/hook family sets it `True`; + a self-hosted HTTP/SSE family (like opencode) sets it `False` and is never handed + a `mux`. +- **The `dev` / `plain` split is a pipeline concept, not a per-family branch.** When + a dev/review session runs the `bmad-dev-auto` skill (which writes no + `result.json`), the engine builds the `dev` variant — the `_DevSynthesisMixin`- + composed class — and threads the project `paths` into it so it can synthesize the + result from the spec on disk; every other role builds `plain`. Both variants of a + family share the `(*args, paths, **kwargs)` dev `__init__` contract, so honoring + it is all an out-of-tree class must do to slot into that machinery. + ### References - [`adapters/opencode_http.py`](../src/bmad_loop/adapters/opencode_http.py) — the diff --git a/src/bmad_loop/adapters/profile.py b/src/bmad_loop/adapters/profile.py index e7d242d0..ec12bdad 100644 --- a/src/bmad_loop/adapters/profile.py +++ b/src/bmad_loop/adapters/profile.py @@ -9,10 +9,24 @@ project-local TOML files in /.bmad-loop/profiles/*.toml overlay them (same name overrides, new names extend) — adding a CLI that clones an existing hook dialect needs no Python. + +An out-of-tree package advertises additional profiles under the +``bmad_loop.profiles`` entry-point group (:func:`load_profiles` scans it): the +companion to the ``bmad_loop.adapters`` registry (:mod:`~.registry`), so a +co-installed adapter package ships both its class and the profile that selects +it with zero project config. Precedence is packaged < entry-point < project (a +project TOML always wins). A broken entry point degrades to a recorded reason +(:func:`external_profile_errors`), never a crash. + +Which adapter *class* drives a profile is the ``adapter`` field, resolved against +the :mod:`~.registry` — it is read here but intentionally **not** validated at +parse time (an unknown kind is caught at construction / by ``validate``, against +the live registry, never a hardcoded set). """ from __future__ import annotations +import importlib.metadata import tomllib from dataclasses import dataclass, field from importlib import resources @@ -54,6 +68,15 @@ class CLIProfile: name: str binary: str hooks: HookSpec + # Which adapter *class* drives this CLI — a key resolved against the adapter + # registry (adapters/registry.py), not a hardcoded enum. "generic" = the + # bundled tmux-injection + hook-signal adapter; "opencode-http" = the bundled + # HTTP/SSE adapter; an out-of-tree package registers its own. Read but NOT + # validated at parse time: an unknown kind fails loud at construction and as a + # `validate` finding, both against the live registry. A hookless HTTP profile + # (hooks.dialect = "none") MUST set this to its HTTP adapter kind — the + # transport (hookless) and the driving class are now decoupled axes. + adapter: str = "generic" # project-relative tree this CLI reads skills from, e.g. ".claude/skills" # (claude) or ".agents/skills" (codex/gemini); `bmad-loop init` installs the # bundled bmad-loop-* skills here. @@ -166,10 +189,17 @@ def fail(msg: str) -> ProfileError: if not seed or is_absolute_path(seed) or has_parent_ref(seed): raise fail(f"seed_files entries must be project-relative paths: got {seed!r}") + # Read but deliberately not validated here: validity of `adapter` is enforced + # against the live adapter registry (at construction and by `validate`), never + # a set literal every new adapter would have to edit. Parsing must stay import- + # cycle-free, so profile.py never imports the registry. + adapter = str(doc.get("adapter", "generic")) + return CLIProfile( name=name, binary=binary, hooks=HookSpec(dialect=dialect, config_path=config_path, events=events), + adapter=adapter, skill_tree=skill_tree, prompt_template=str(doc.get("prompt_template", "{prompt}")), launch_args=tuple(str(a) for a in doc.get("launch_args", ())), @@ -193,14 +223,83 @@ def _load_toml(text: str, source: str) -> CLIProfile: return _parse_profile(doc, source) +# The entry-point group an out-of-tree package advertises extra profiles under — +# the companion to adapters/registry.py's `bmad_loop.adapters` group. Each entry +# point loads to a provider: a callable returning an iterable of CLIProfile (or an +# iterable directly), e.g. one built from the package's own bundled TOML. Scanned +# once per process (a third-party import failure is not transient); the resulting +# profiles are process-global (project-independent), so only the project overlay +# is re-read per load_profiles call. A broken entry point is recorded, not raised. +PROFILES_GROUP = "bmad_loop.profiles" +_EXTERNALS_LOADED = False +_EXTERNAL_PROFILES: dict[str, CLIProfile] = {} +_PROFILE_LOAD_ERRORS: dict[str, str] = {} + + +def _coerce_profiles(produced: object, ep_name: str) -> list[CLIProfile]: + """A provider may return a callable's result or an iterable directly; either + way it must yield CLIProfile instances. Anything else is the package's bug — + reported (per :func:`external_profile_errors`), never trusted into the map.""" + try: + items = list(produced) + except TypeError as exc: + raise ProfileError( + f"{ep_name}: profile provider must return an iterable of CLIProfile" + ) from exc + for item in items: + if not isinstance(item, CLIProfile): + raise ProfileError( + f"{ep_name}: profile provider yielded {type(item).__name__}, not CLIProfile" + ) + return items + + +def _load_external_profiles() -> dict[str, CLIProfile]: + """Import every ``bmad_loop.profiles`` entry point and collect the profiles it + provides, first-registration-wins on a name collision. Scan-once; failures are + recorded in ``_PROFILE_LOAD_ERRORS`` (surfaced via + :func:`external_profile_errors`), never raised — a broken adapter package must + not break profile loading for everything else.""" + global _EXTERNALS_LOADED + if _EXTERNALS_LOADED: + return _EXTERNAL_PROFILES + _EXTERNALS_LOADED = True + try: + eps = importlib.metadata.entry_points(group=PROFILES_GROUP) + except Exception as exc: # noqa: BLE001 — diagnostics path, never crash loading + _PROFILE_LOAD_ERRORS[""] = f"{type(exc).__name__}: {exc}" + return _EXTERNAL_PROFILES + for ep in eps: + try: + provider = ep.load() + produced = provider() if callable(provider) else provider + for profile in _coerce_profiles(produced, ep.name): + _EXTERNAL_PROFILES.setdefault(profile.name, profile) + except Exception as exc: # noqa: BLE001 — one bad package must not hide the rest + _PROFILE_LOAD_ERRORS[ep.name] = f"{type(exc).__name__}: {exc}" + return _EXTERNAL_PROFILES + + +def external_profile_errors() -> dict[str, str]: + """Entry-point name -> failure reason for every external profile provider that + failed to load this process (empty when all loaded). For diagnostics surfaces.""" + return dict(_PROFILE_LOAD_ERRORS) + + def load_profiles(project: Path | None = None) -> dict[str, CLIProfile]: - """Packaged built-ins, overlaid by /.bmad-loop/profiles/*.toml.""" + """Packaged built-ins, overlaid by ``bmad_loop.profiles`` entry-point + profiles, overlaid by /.bmad-loop/profiles/*.toml. + + Precedence is packaged < entry-point < project: a co-installed adapter package + extends (or overrides) the bundled set, and a project TOML always wins.""" profiles: dict[str, CLIProfile] = {} packaged = resources.files("bmad_loop.data").joinpath("profiles") for entry in sorted(packaged.iterdir(), key=lambda e: e.name): if entry.name.endswith(".toml"): profile = _load_toml(entry.read_text(encoding="utf-8"), entry.name) profiles[profile.name] = profile + for name, profile in _load_external_profiles().items(): + profiles[name] = profile if project is not None: user_dir = project / USER_PROFILES_REL if user_dir.is_dir(): diff --git a/src/bmad_loop/adapters/registry.py b/src/bmad_loop/adapters/registry.py new file mode 100644 index 00000000..c4988f9b --- /dev/null +++ b/src/bmad_loop/adapters/registry.py @@ -0,0 +1,259 @@ +"""Coding-CLI adapter registry — the out-of-tree extension seam for the CLI axis. + +The transport axis (:mod:`~.multiplexer`) has long been extensible out-of-tree: +a backend registers through ``register_multiplexer`` and a co-installed package +is discovered via the ``bmad_loop.mux_backends`` entry-point group. This module +is the same seam for the *other* axis — which adapter **class** drives a coding +CLI. A CLI that fits the tmux-injection + hook-signal transport still needs no +Python at all (drop a TOML :class:`~.profile.CLIProfile`); this registry is for a +CLI that needs a whole new adapter subclass (the shipped example is the HTTP/SSE +``opencode-http`` adapter, which no tmux profile can host). + +An adapter *kind* is selected by **data**: ``profile.adapter`` names the kind, and +:func:`get_adapter_kind` resolves it. Two registration-time dataclasses carry the +family: + +- :class:`AdapterKind` — ``name`` + ``needs_mux`` (does the family drive a + terminal multiplexer?) + ``load``, a lazy thunk returning the builder. The thunk + is why registration, validation (``known_adapter_kinds``) and listing + (``detect_adapters``) never import a heavy adapter module — nor an optional + dependency like ``httpx``, which only the opencode family pulls in at + construction. +- :class:`AdapterBuilder` — the ``plain`` class, the ``_DevSynthesisMixin``-composed + ``dev`` class (both share the ``(*args, paths, **kwargs)`` dev ``__init__``), and + the family's construction-failure exception type(s) (``()`` = none; + ``(OpencodeServerError,)`` for the HTTP family, which fails loud when its server + can't spawn). ``cli._make_adapters`` converts a raised ``construct_error`` into a + ``SystemExit``. + +Bundled kinds register from :func:`_load_builtin_adapters` (``generic``, +``opencode-http``); out-of-tree kinds arrive at import time, triggered by the +``bmad_loop.adapters`` entry-point scan in :func:`_load_external_adapters` — so a +pip/uv co-installed adapter package is selectable with no config step. Builtins +load first, so an external can never shadow a bundled name. A broken third-party +distribution degrades to a recorded, surfaced reason +(:func:`external_adapter_errors`) and can never break selection. + +**Two deliberate asymmetries versus the multiplexer seam** (this is not a +copy-paste omission): + +- *No process-wide cache / no ``cache_clear``.* The multiplexer is a single + process-wide singleton behind an ``lru_cache``; adapters are built **per run** + in ``cli._make_adapters``, which keeps its own ``by_cfg`` cache keyed on + ``(resolved-config, synthesizes)``. Selection here is a pure registry lookup, so + there is nothing to cache and :func:`register_adapter` invalidates nothing. +- *No ``configure_*`` / ``matches(platform)`` / platform defaults.* The multiplexer + is chosen by a policy knob and a ``sys.platform`` predicate; an adapter kind is + chosen by the ``profile.adapter`` field alone. There is no host-dependent + auto-selection and no persisted choice to install, so none of that machinery + exists — ``get_adapter_kind(name)`` fails loud on an unknown name rather than + falling back. +""" + +from __future__ import annotations + +import importlib.metadata +from collections.abc import Callable +from dataclasses import dataclass + + +class AdapterError(Exception): + """An adapter kind could not be resolved (an unknown ``profile.adapter``). + + Construction failures a *known* family raises during ``__init__`` are its own + types (e.g. ``OpencodeServerError``), carried by + :attr:`AdapterBuilder.construct_error`, not this seam-level type.""" + + +@dataclass(frozen=True) +class AdapterBuilder: + """The classes and failure modes of one adapter family. + + ``plain`` and ``dev`` are the two variants ``cli._make_adapters`` picks + between on the ``synthesizes`` axis (a ``bmad-dev-auto`` dev/review session + gets ``dev``, which takes an extra ``paths=`` kwarg; every other role gets + ``plain``). ``construct_error`` is the tuple of exception types the family's + constructor raises when the session cannot be built — empty for a family that + cannot fail construction (``generic``); the caller wraps a match into a + ``SystemExit`` so a run aborts with a clean message instead of a traceback.""" + + plain: type + dev: type + construct_error: tuple[type[BaseException], ...] = () + + +@dataclass(frozen=True) +class AdapterKind: + """One registered adapter family, keyed by ``name`` (the ``profile.adapter`` + value). ``needs_mux`` gates whether ``cli._make_adapters`` resolves and + usability-checks the shared terminal multiplexer for this family (a hookless + HTTP/SSE family needs no transport). ``load`` is a lazy thunk returning the + :class:`AdapterBuilder`; it is the *only* place the family's classes (and any + optional dependency they pull in) are imported.""" + + name: str + needs_mux: bool + load: Callable[[], AdapterBuilder] + + +# ---------------------------------------------------------------- builtins + + +def _generic_builder() -> AdapterBuilder: + from .generic import GenericAdapter, GenericDevAdapter + + return AdapterBuilder(plain=GenericAdapter, dev=GenericDevAdapter, construct_error=()) + + +def _opencode_http_builder() -> AdapterBuilder: + from .opencode_http import ( + OpencodeDevAdapter, + OpencodeHttpAdapter, + OpencodeServerError, + ) + + return AdapterBuilder( + plain=OpencodeHttpAdapter, + dev=OpencodeDevAdapter, + construct_error=(OpencodeServerError,), + ) + + +# The bundled kinds, as (name, needs_mux, load-thunk). A module constant, not +# mutable registry state, so detect_adapters can label a row builtin-vs-external +# without the fixtures having to snapshot it. `generic` drives tmux + hooks and +# needs the multiplexer; `opencode-http` is hookless HTTP/SSE and does not. +_BUILTIN_ADAPTERS: tuple[tuple[str, bool, Callable[[], AdapterBuilder]], ...] = ( + ("generic", True, _generic_builder), + ("opencode-http", False, _opencode_http_builder), +) +_BUILTIN_NAMES = frozenset(name for name, _, _ in _BUILTIN_ADAPTERS) + +# The live registry: name -> AdapterKind. Unlike the multiplexer's ordered list +# (registration order breaks selection ties), adapter selection is a pure by-name +# lookup, so a dict with first-registration-wins semantics is the whole story. +_ADAPTERS: dict[str, AdapterKind] = {} +_BUILTINS_LOADED = False + + +def register_adapter(name: str, needs_mux: bool, load: Callable[[], AdapterBuilder]) -> None: + """Register an adapter kind. ``name`` is the ``profile.adapter`` key that + selects it; ``needs_mux`` declares whether the family drives a terminal + multiplexer; ``load`` is the lazy builder thunk. First registration of a name + wins — bundled kinds register from :func:`_load_builtin_adapters` before the + entry-point scan, so an out-of-tree package can never shadow a bundled name. + An out-of-tree kind calls this at import time — no core edit required. There + is no selection cache to invalidate (see the module docstring).""" + _ADAPTERS.setdefault(name, AdapterKind(name=name, needs_mux=needs_mux, load=load)) + + +def _load_builtin_adapters() -> None: + """Register the bundled adapter kinds. Idempotent and lazy (called from the + resolution entry points, not at module import) to stay cycle-safe: the load + thunks import ``generic`` / ``opencode_http``, which import back through the + package. Builtins register before externals so a bundled name keeps + first-wins on any collision.""" + global _BUILTINS_LOADED + if _BUILTINS_LOADED: + return + for name, needs_mux, load in _BUILTIN_ADAPTERS: + register_adapter(name, needs_mux, load) + _BUILTINS_LOADED = True + + +# The entry-point group an out-of-tree adapter package advertises its module +# under; importing the module runs its register_adapter call. Loader state: +# scanned-once flag + per-entry-point failure reasons for adapters/validate. +ADAPTERS_GROUP = "bmad_loop.adapters" +_EXTERNALS_LOADED = False +_EXTERNAL_ERRORS: dict[str, str] = {} + + +def _load_external_adapters() -> None: + """Import every ``bmad_loop.adapters`` entry point; each module self-registers + via :func:`register_adapter` at import time. Called after + :func:`_load_builtin_adapters`, so builtins keep first registration. + + A broken third-party distribution must never break adapter selection: + failures are recorded in ``_EXTERNAL_ERRORS`` (surfaced by ``bmad-loop + adapters`` and the ``validate`` preflight via :func:`external_adapter_errors`), + not raised. The loaded-flag is set up front: a third-party import failure is + not transient, and retrying on every resolution would re-import (and re-fail) + each time — mirroring the multiplexer's external scan.""" + global _EXTERNALS_LOADED + if _EXTERNALS_LOADED: + return + _EXTERNALS_LOADED = True + try: + eps = importlib.metadata.entry_points(group=ADAPTERS_GROUP) + except Exception as exc: # noqa: BLE001 — diagnostics path, never crash selection + _EXTERNAL_ERRORS[""] = f"{type(exc).__name__}: {exc}" + return + for ep in eps: + try: + ep.load() # module import runs register_adapter(...) + except Exception as exc: # noqa: BLE001 — one bad package must not hide the rest + _EXTERNAL_ERRORS[ep.name] = f"{type(exc).__name__}: {exc}" + + +def external_adapter_errors() -> dict[str, str]: + """Entry-point name -> failure reason for every external adapter that failed + to load this process (empty when all loaded). For diagnostics surfaces.""" + return dict(_EXTERNAL_ERRORS) + + +def _known() -> str: + return ", ".join(sorted(_ADAPTERS)) or "(none registered)" + + +def get_adapter_kind(name: str) -> AdapterKind: + """Resolve the adapter kind named ``name`` (a ``profile.adapter`` value), + loading the builtins and scanning the entry-point group first. + + Fails loud on an unknown name, listing the registered kinds — an explicit but + unregistered adapter is a misconfiguration (a typo, or a plugin package that + isn't installed), never something to silently fall back from. The caller + (``cli._make_adapters``) adds the offending profile's name to the message.""" + _load_builtin_adapters() + _load_external_adapters() + kind = _ADAPTERS.get(name) + if kind is None: + raise AdapterError(f"unknown adapter kind {name!r}; known: {_known()}") + return kind + + +def known_adapter_kinds() -> list[str]: + """Sorted names of every registered adapter kind (builtins + successfully + loaded externals). The oracle for ``validate``'s ``adapter.kind`` finding — + validity of ``profile.adapter`` is enforced against this registry, never a + hardcoded set.""" + _load_builtin_adapters() + _load_external_adapters() + return sorted(_ADAPTERS) + + +@dataclass(frozen=True) +class AdapterKindInfo: + """One registered adapter kind's detection row, for ``bmad-loop adapters`` and + the ``validate`` preflight.""" + + name: str + needs_mux: bool + builtin: bool + + +def detect_adapters() -> list[AdapterKindInfo]: + """Enumerate every registered adapter kind (builtins + loaded externals), + sorted by name, each labelled builtin-vs-external. Never raises — this feeds + diagnostics, which must work on a misconfigured host. The ``load`` thunk is + never invoked, so listing stays free of any heavy adapter import.""" + _load_builtin_adapters() + _load_external_adapters() + return [ + AdapterKindInfo( + name=kind.name, + needs_mux=kind.needs_mux, + builtin=kind.name in _BUILTIN_NAMES, + ) + for kind in sorted(_ADAPTERS.values(), key=lambda k: k.name) + ] diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index 739841bb..7a18b0e0 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -43,6 +43,9 @@ "adapter.binary", "adapter.hookless", "adapter.httpx", + "adapter.kind", + "adapter.external", + "adapter.external-profile", "queue.sprint-status", "queue.sprint-status-unknown-keys", "queue.stories-manifest", diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index e2dcba22..7ba80696 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -123,9 +123,9 @@ def _reconcile_stale(project: Path, paths: bmadconfig.ProjectPaths, pol) -> None def _make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIAdapter]: - from .adapters.generic import GenericAdapter, GenericDevAdapter from .adapters.multiplexer import get_multiplexer, mux_usable from .adapters.profile import ProfileError, get_profile + from .adapters.registry import AdapterError, get_adapter_kind # The dev skill (bmad-dev-auto) writes no result.json: its adapter # synthesizes the result from the spec, and so needs the project paths to @@ -141,7 +141,10 @@ def _make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIA # session re-invokes the dev skill on the done spec for a follow-up pass), # and the skill writes no result.json — its adapter synthesizes the result # from the spec it leaves on disk, so it needs the project paths to find - # that spec and cannot be shared with the triage role even on identical config. + # that spec and cannot be shared with the triage role even on identical + # config. `synthesizes` is a bmad-dev-auto pipeline concept (which variant + # of a family to build + whether to thread `paths`), NOT a per-family + # branch — it stays a documented contract for every registered adapter. synthesizes = role in ("dev", "review") and policy.dev.skill == "bmad-dev-auto" key = (cfg, synthesizes) if key not in by_cfg: @@ -149,34 +152,27 @@ def _make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIA profile = get_profile(cfg.name, project) except ProfileError as e: raise SystemExit(f"error: {e}") from e - if profile.hookless: - # Hookless profiles (opencode-http) are driven over HTTP/SSE — - # the tmux adapters below cannot host them. - from .adapters.opencode_http import ( - OpencodeDevAdapter, - OpencodeHttpAdapter, - OpencodeServerError, - ) - - common = dict( - run_dir=run_dir, - policy=policy, - profile=profile, - extra_args=cfg.extra_args, - usage_grace_s=cfg.usage_grace_s, - stop_without_result_nudges=cfg.stop_without_result_nudges, - ) - try: - by_cfg[key] = ( - OpencodeDevAdapter(**common, paths=paths) - if synthesizes - else OpencodeHttpAdapter(**common) - ) - except OpencodeServerError as e: - raise SystemExit(f"error: {e}") from e - else: - # Resolve and probe the shared multiplexer only when a profile - # actually uses it; hookless HTTP/SSE runs need no transport. + # Which adapter class drives this CLI is pure data — profile.adapter + # resolved against the registry. No adapter-name branching lives here; + # a new family (Hermes, herdr, ...) plugs in with zero edits to this + # function. An unknown kind fails loud naming the profile. + try: + kind = get_adapter_kind(profile.adapter) + except AdapterError as e: + raise SystemExit(f"error: profile {profile.name!r}: {e}") from e + builder = kind.load() + common = dict( + run_dir=run_dir, + policy=policy, + profile=profile, + extra_args=cfg.extra_args, + usage_grace_s=cfg.usage_grace_s, + stop_without_result_nudges=cfg.stop_without_result_nudges, + ) + if kind.needs_mux: + # Resolve and probe the shared multiplexer only when a kind + # actually drives one; hookless HTTP/SSE families need no + # transport (and monkeypatched tests assert it is never touched). if mux is None: mux = get_multiplexer() if not mux_usable(mux): @@ -190,20 +186,17 @@ def _make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIA "missing, the version is unsupported, or a required helper is " "absent (psmux needs `pwsh` on PATH); see `bmad-loop diagnose`" ) - common = dict( - run_dir=run_dir, - policy=policy, - profile=profile, - extra_args=cfg.extra_args, - usage_grace_s=cfg.usage_grace_s, - stop_without_result_nudges=cfg.stop_without_result_nudges, - mux=mux, - ) - by_cfg[key] = ( - GenericDevAdapter(**common, paths=paths) - if synthesizes - else GenericAdapter(**common) - ) + common["mux"] = mux + # The synthesizing variant additionally needs `paths`; the plain + # variant does not accept it. construct_error is family-declared — + # `()` for a family that can't fail construction (generic), or e.g. + # `(OpencodeServerError,)` for one that can — and becomes a SystemExit. + cls = builder.dev if synthesizes else builder.plain + build_kwargs = {**common, "paths": paths} if synthesizes else common + try: + by_cfg[key] = cls(**build_kwargs) + except builder.construct_error as e: + raise SystemExit(f"error: {e}") from e adapters[role] = by_cfg[key] return adapters @@ -359,7 +352,7 @@ def cmd_validate(args: argparse.Namespace) -> int: # story-queue gate runs below: the sprint-status file (sprint mode) or the # stories.yaml manifest (stories mode). Loaded before the queue gate so a # stories-only project is not failed on a missing sprint-status.yaml. - from .adapters.profile import ProfileError, get_profile + from .adapters.profile import ProfileError, external_profile_errors, get_profile profiles = [] profile_by_name: dict[str, object] = {} @@ -477,6 +470,42 @@ def cmd_validate(args: argparse.Namespace) -> int: {"profile": profile.name, "config_path": str(hook_config)}, ) + # Adapter-kind validity is enforced against the LIVE registry, never a + # hardcoded set: a profile.adapter naming no registered kind is a config error + # (a typo, or an uninstalled plugin package). External adapter/profile packages + # that failed to load are surfaced as warnings — selection already degraded + # past them (the same non-blocking treatment as a failed mux backend package). + from .adapters.registry import external_adapter_errors, known_adapter_kinds + + kinds = known_adapter_kinds() + for profile in profiles: + if profile.adapter in kinds: + report.ok( + "adapter.kind", + f"{profile.name}: adapter kind {profile.adapter!r} registered", + {"profile": profile.name, "adapter": profile.adapter}, + ) + else: + report.fail( + "adapter.kind", + f"{profile.name}: unknown adapter kind {profile.adapter!r} — " + f"known: {', '.join(kinds)} (install the plugin that provides it, " + f"or fix the profile's `adapter`)", + {"profile": profile.name, "adapter": profile.adapter, "known": kinds}, + ) + for ep_name, reason in sorted(external_adapter_errors().items()): + report.warn( + "adapter.external", + f"external adapter '{ep_name}' failed to load: {reason}", + {"entry_point": ep_name, "error": reason}, + ) + for ep_name, reason in sorted(external_profile_errors().items()): + report.warn( + "adapter.external-profile", + f"external profile '{ep_name}' failed to load: {reason}", + {"entry_point": ep_name, "error": reason}, + ) + # opencode config-file model ids are "provider/model" (see the opencode_http docstring); # a bare model name silently falls back to the server's default model, so warn # (advisory — a note, not a FAIL: an empty model legitimately means "default"). @@ -628,6 +657,57 @@ def _mux_set(project: Path, args: argparse.Namespace) -> int: return 0 +def cmd_adapters(args: argparse.Namespace) -> int: + """List the registered coding-CLI adapter kinds (the CLI axis's counterpart to + `bmad-loop mux` for the transport axis) and name any out-of-tree adapter or + profile package that failed to load. Unlike `mux`, there is no global choice + to persist: an adapter kind is selected per profile by its `adapter` field.""" + from .adapters.profile import external_profile_errors, load_profiles + from .adapters.registry import detect_adapters, external_adapter_errors + + # Loading profiles also triggers the bmad_loop.profiles entry-point scan, so a + # broken profile package surfaces below alongside a broken adapter package. + profiles = load_profiles(_project(args)) + by_kind: dict[str, list[str]] = {} + for prof in profiles.values(): + by_kind.setdefault(prof.adapter, []).append(prof.name) + + rows = detect_adapters() + header = ("NAME", "ORIGIN", "NEEDS MUX", "PROFILES") + table = [ + ( + r.name, + "builtin" if r.builtin else "external", + "yes" if r.needs_mux else "no", + ", ".join(sorted(by_kind.get(r.name, []))) or "-", + ) + for r in rows + ] + widths = [max(len(h), *(len(row[i]) for row in table), 0) for i, h in enumerate(header)] + for row in (header, *table): + print(" ".join(cell.ljust(w) for cell, w in zip(row, widths)).rstrip()) + # A profile whose adapter kind never registered (a typo, or an uninstalled + # plugin) is invisible in the table above — name it so an operator can see the + # dangling reference, exactly as `mux` names a failed backend package. + known = {r.name for r in rows} + for kind in sorted(set(by_kind) - known): + print( + f"warning: profile(s) {', '.join(sorted(by_kind[kind]))} reference unknown " + f"adapter kind '{kind}' (no registered kind or plugin provides it)", + file=sys.stderr, + ) + for ep_name, reason in sorted(external_adapter_errors().items()): + print(f"warning: external adapter '{ep_name}' failed to load: {reason}", file=sys.stderr) + for ep_name, reason in sorted(external_profile_errors().items()): + print(f"warning: external profile '{ep_name}' failed to load: {reason}", file=sys.stderr) + print( + "adapter kind is selected per profile by its `adapter` field " + "(default: generic); an out-of-tree package registers new kinds via the " + "bmad_loop.adapters + bmad_loop.profiles entry points" + ) + return 0 + + 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). @@ -2324,6 +2404,12 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: "backend that only registers on the target machine)", ) + add( + "adapters", + cmd_adapters, + "list registered coding-CLI adapter kinds + which profiles select them", + ) + probe_p = add( "probe-adapter", cmd_probe, diff --git a/src/bmad_loop/data/profiles/opencode.toml b/src/bmad_loop/data/profiles/opencode.toml index efd1510c..7948ba50 100644 --- a/src/bmad_loop/data/profiles/opencode.toml +++ b/src/bmad_loop/data/profiles/opencode.toml @@ -4,6 +4,9 @@ # API facts pinned against opencode 1.18.2 (2026-07-16): see the adapters/opencode_http.py docstring. name = "opencode-http" binary = "opencode" +# Driven by the bundled HTTP/SSE adapter class (adapters/registry.py), not the +# tmux generic adapter — the selector for the coding-CLI adapter axis. +adapter = "opencode-http" prompt_template = "Use the {skill} skill now: {args}" usage_parser = "none" skill_tree = ".claude/skills" diff --git a/tests/test_adapter_registry.py b/tests/test_adapter_registry.py new file mode 100644 index 00000000..82b60d66 --- /dev/null +++ b/tests/test_adapter_registry.py @@ -0,0 +1,496 @@ +"""Coding-CLI adapter-registry selection + discovery proof. + +The CLI axis selects its adapter *class* through a registry +(:func:`~bmad_loop.adapters.registry.register_adapter`) keyed on the +``profile.adapter`` field, rather than name-branching in ``cli._make_adapters``. +So a new adapter family is a registration — not a core edit — exactly as a new +transport backend is (:mod:`bmad_loop.adapters.multiplexer`). These tests pin the +registry (builtin + external registration, builtins-first-wins, unknown-kind +fail-loud), the out-of-tree entry-point discovery and its degrade-not-crash +contract, and the ``cli._make_adapters`` dispatch: a registered kind builds, the +``(cfg, synthesizes)`` cache shares instances, ``needs_mux`` gates the transport +resolution, a construction failure becomes a clean ``SystemExit``, and — the +regression pin — ``opencode-http`` still dispatches to the HTTP adapters unchanged. + +The registry is deliberately simpler than the multiplexer seam: adapters are built +per-run (``cli._make_adapters`` owns the cache), so there is no ``lru_cache`` / +``cache_clear`` and no ``configure_*`` / platform-default machinery — the +``fresh_adapter_registry`` fixture snapshots only ``_ADAPTERS`` / the two loaded +flags / ``_EXTERNAL_ERRORS``. + +Entry points are faked by monkeypatching ``importlib.metadata.entry_points`` +through the ``registry`` module's own binding; one test builds a real +``*.dist-info`` on ``sys.path`` to prove the scan works against genuine packaging +metadata. +""" + +from __future__ import annotations + +import pytest +from conftest import install_bmad_config + +from bmad_loop import cli +from bmad_loop import policy as policy_mod +from bmad_loop.adapters import multiplexer as mux_mod +from bmad_loop.adapters import profile as profile_mod +from bmad_loop.adapters import registry as m +from bmad_loop.adapters.registry import AdapterBuilder, AdapterError + +# --------------------------------------------------------------------------- # +# Stubs + fixtures + + +class _StubAdapter: + """Plain-variant double: records the kwargs cli._make_adapters passes so a + test can assert the mux was (or was not) threaded in.""" + + def __init__(self, **kwargs): + self.kwargs = kwargs + + +class _StubDevAdapter(_StubAdapter): + """Dev-variant double: mirrors the real ``(*args, paths, **kwargs)`` dev + __init__ contract every registered family's dev class honors.""" + + def __init__(self, *, paths, **kwargs): + super().__init__(**kwargs) + self.paths = paths + + +def _stub_builder(*, needs_mux_construct_error=()): + return AdapterBuilder( + plain=_StubAdapter, dev=_StubDevAdapter, construct_error=needs_mux_construct_error + ) + + +class _FakeEntryPoint: + """Duck-typed importlib.metadata.EntryPoint: the loader only touches + ``.name`` and ``.load()``.""" + + def __init__(self, name, load): + self.name = name + self._load = load + + def load(self): + return self._load() + + +@pytest.fixture +def fresh_adapter_registry(monkeypatch): + """Isolate the global adapter registry: snapshot, clear, restore. No + lru_cache / configured-choice to reset — the deliberate asymmetry vs. the mux + seam. The externals scan is parked as already-loaded so whatever adapters are + installed on the dev box can't leak into builtin tests (discovery tests re-arm + it via :func:`scan_adapter_registry`). The companion profile-module external + scan is suppressed too, so an installed ``bmad_loop.profiles`` package can't + leak a profile into the ``_make_adapters`` integration tests.""" + saved_adapters = dict(m._ADAPTERS) + saved_loaded = m._BUILTINS_LOADED + saved_ext_loaded = m._EXTERNALS_LOADED + saved_ext_errors = dict(m._EXTERNAL_ERRORS) + m._ADAPTERS.clear() + m._BUILTINS_LOADED = False + m._EXTERNALS_LOADED = True # externals OFF by default; discovery tests opt back in + m._EXTERNAL_ERRORS.clear() + + saved_prof_loaded = profile_mod._EXTERNALS_LOADED + saved_prof_profiles = dict(profile_mod._EXTERNAL_PROFILES) + saved_prof_errors = dict(profile_mod._PROFILE_LOAD_ERRORS) + profile_mod._EXTERNALS_LOADED = True + profile_mod._EXTERNAL_PROFILES.clear() + profile_mod._PROFILE_LOAD_ERRORS.clear() + + yield m + + m._ADAPTERS.clear() + m._ADAPTERS.update(saved_adapters) + m._BUILTINS_LOADED = saved_loaded + m._EXTERNALS_LOADED = saved_ext_loaded + m._EXTERNAL_ERRORS.clear() + m._EXTERNAL_ERRORS.update(saved_ext_errors) + profile_mod._EXTERNALS_LOADED = saved_prof_loaded + profile_mod._EXTERNAL_PROFILES.clear() + profile_mod._EXTERNAL_PROFILES.update(saved_prof_profiles) + profile_mod._PROFILE_LOAD_ERRORS.clear() + profile_mod._PROFILE_LOAD_ERRORS.update(saved_prof_errors) + + +@pytest.fixture +def scan_adapter_registry(fresh_adapter_registry, monkeypatch): + """fresh_adapter_registry with the externals scan re-armed. Yields a hook: + call it with fake entry points (or a ``scan_error`` to raise from the scan + itself) and the next resolution performs that scan.""" + + def arm(*eps, scan_error=None): + def fake_entry_points(*, group): + assert group == m.ADAPTERS_GROUP + if scan_error is not None: + raise scan_error + return list(eps) + + monkeypatch.setattr(m.importlib.metadata, "entry_points", fake_entry_points) + m._EXTERNALS_LOADED = False + m._EXTERNAL_ERRORS.clear() + + yield fresh_adapter_registry, arm + + +def _write_policy(project, text) -> None: + d = project / ".bmad-loop" + d.mkdir(parents=True, exist_ok=True) + (d / "policy.toml").write_text(text, encoding="utf-8") + + +def _write_profile(project, name, *, adapter, hookless=True) -> None: + d = project / ".bmad-loop" / "profiles" + d.mkdir(parents=True, exist_ok=True) + if hookless: + hooks = '[hooks]\ndialect = "none"\n' + else: + hooks = '[hooks]\ndialect = "claude-settings-json"\nconfig_path = ".hermes/settings.json"\n[hooks.events]\nStop = "Stop"\n' + (d / f"{name}.toml").write_text( + f'name = "{name}"\nbinary = "{name}"\nadapter = "{adapter}"\n{hooks}', encoding="utf-8" + ) + + +def _run_dir(project): + return project / ".bmad-loop" / "runs" / "r" + + +# --------------------------------------------------------------------------- # +# Registry — builtin registration + fail-loud + + +def test_builtin_kinds_registered_with_needs_mux(fresh_adapter_registry): + """The two bundled kinds register with the correct transport requirement: + generic drives tmux (needs_mux), opencode-http is hookless HTTP/SSE (does not).""" + generic = fresh_adapter_registry.get_adapter_kind("generic") + http = fresh_adapter_registry.get_adapter_kind("opencode-http") + assert generic.needs_mux is True + assert http.needs_mux is False + assert fresh_adapter_registry.known_adapter_kinds() == ["generic", "opencode-http"] + + +def test_load_thunk_returns_real_builder(fresh_adapter_registry): + """The lazy thunk resolves to the real adapter classes — imported only now, + never at registration/listing time.""" + from bmad_loop.adapters.generic import GenericAdapter, GenericDevAdapter + + builder = fresh_adapter_registry.get_adapter_kind("generic").load() + assert builder.plain is GenericAdapter + assert builder.dev is GenericDevAdapter + assert builder.construct_error == () + + +def test_unknown_kind_fails_loud_naming_known(fresh_adapter_registry): + """An explicit but unregistered kind is a misconfiguration: fail loud, listing + the registered kinds — never silently fall back.""" + with pytest.raises(AdapterError, match=r"nonesuch.*generic.*opencode-http"): + fresh_adapter_registry.get_adapter_kind("nonesuch") + + +def test_register_adapter_first_wins(fresh_adapter_registry): + """A duplicate registration of a name is ignored (first wins) — the mechanism + that lets builtins load first and shrug off a same-named external.""" + registry = fresh_adapter_registry + registry.register_adapter("dup", needs_mux=True, load=lambda: _stub_builder()) + registry.register_adapter("dup", needs_mux=False, load=lambda: _stub_builder()) + assert registry.get_adapter_kind("dup").needs_mux is True + + +def test_detect_adapters_labels_builtin(fresh_adapter_registry): + """detect_adapters lists every kind, sorted, labelled builtin-vs-external, + without invoking any load thunk (no heavy import).""" + registry = fresh_adapter_registry + registry.register_adapter("extra", needs_mux=False, load=lambda: _stub_builder()) + rows = {r.name: r for r in registry.detect_adapters()} + assert set(rows) == {"generic", "opencode-http", "extra"} + assert rows["generic"].builtin is True and rows["generic"].needs_mux is True + assert rows["opencode-http"].builtin is True and rows["opencode-http"].needs_mux is False + assert rows["extra"].builtin is False + assert [r.name for r in registry.detect_adapters()] == sorted(rows) + + +# --------------------------------------------------------------------------- # +# External discovery (the bmad_loop.adapters entry-point scan) + + +def test_entry_point_adapter_registers_and_is_selectable(scan_adapter_registry): + """The pip-install-and-go path: the entry point's module import registers the + kind; it resolves and lists like a builtin, with no load error.""" + registry, arm = scan_adapter_registry + + def load(): + registry.register_adapter("extadapter", needs_mux=False, load=lambda: _stub_builder()) + + arm(_FakeEntryPoint("extadapter", load)) + kind = registry.get_adapter_kind("extadapter") + assert kind.needs_mux is False + assert "extadapter" in {r.name for r in registry.detect_adapters()} + assert registry.external_adapter_errors() == {} + + +def test_builtins_first_wins_over_external(scan_adapter_registry): + """Ordering guarantee: builtins register before the scan, so an external that + tries to register the bundled name ``generic`` cannot shadow it.""" + registry, arm = scan_adapter_registry + + def load(): + # a hostile/clumsy external claiming the builtin name with wrong needs_mux + registry.register_adapter("generic", needs_mux=False, load=lambda: _stub_builder()) + + arm(_FakeEntryPoint("shadow", load)) + kind = registry.get_adapter_kind("generic") + assert kind.needs_mux is True # the builtin, not the external + assert kind.load().plain.__name__ == "GenericAdapter" + + +def test_broken_entry_point_degrades_and_is_recorded(scan_adapter_registry): + """A distribution whose import blows up must not break selection: the builtins + still resolve, and the failure is recorded for adapters/validate to show.""" + registry, arm = scan_adapter_registry + + def boom(): + raise ImportError("No module named 'ghost_dependency'") + + arm(_FakeEntryPoint("brokenadapter", boom)) + assert registry.get_adapter_kind("generic").needs_mux is True # selection still works + errors = registry.external_adapter_errors() + assert list(errors) == ["brokenadapter"] + assert "ghost_dependency" in errors["brokenadapter"] + + +def test_one_broken_package_does_not_hide_the_rest(scan_adapter_registry): + """Per-entry isolation: the loader keeps importing after a failure, so a + working adapter still registers alongside a broken one.""" + registry, arm = scan_adapter_registry + + def boom(): + raise RuntimeError("half-installed") + + def load(): + registry.register_adapter("goodadapter", needs_mux=True, load=lambda: _stub_builder()) + + arm(_FakeEntryPoint("brokenadapter", boom), _FakeEntryPoint("goodadapter", load)) + assert registry.get_adapter_kind("goodadapter").needs_mux is True + assert list(registry.external_adapter_errors()) == ["brokenadapter"] + + +def test_scan_failure_degrades(scan_adapter_registry): + """Even the entry-point enumeration itself blowing up leaves selection working, + with the scan failure recorded.""" + registry, arm = scan_adapter_registry + arm(scan_error=RuntimeError("metadata index corrupt")) + assert registry.get_adapter_kind("generic").needs_mux is True + assert "" in registry.external_adapter_errors() + + +def test_scan_runs_once_per_process(scan_adapter_registry): + """The loaded-flag is set up front: a second resolution does not re-scan (a + third-party import failure is not transient; re-importing would re-fail).""" + registry, arm = scan_adapter_registry + calls = [] + + def load(): + calls.append(1) + + arm(_FakeEntryPoint("extadapter", load)) + registry.known_adapter_kinds() + registry.known_adapter_kinds() + assert len(calls) == 1 + + +def test_real_dist_info_metadata_is_discovered(fresh_adapter_registry, monkeypatch, tmp_path): + """End-to-end against genuine packaging metadata: a real ``*.dist-info`` + + module on sys.path is found by the unpatched importlib scan and its import + registers the kind — proving the group name works outside our fakes.""" + site = tmp_path / "site" + site.mkdir() + (site / "extadapter_pkg.py").write_text( + "from bmad_loop.adapters.registry import register_adapter\n" + "def _load():\n" + " raise AssertionError('load thunk must stay lazy')\n" + "register_adapter('extadapter-real', False, _load)\n", + encoding="utf-8", + ) + dist = site / "extadapter-0.1.dist-info" + dist.mkdir() + (dist / "METADATA").write_text("Metadata-Version: 2.1\nName: extadapter\nVersion: 0.1\n") + (dist / "entry_points.txt").write_text( + "[bmad_loop.adapters]\nextadapter = extadapter_pkg\n", encoding="utf-8" + ) + monkeypatch.syspath_prepend(str(site)) + fresh_adapter_registry._EXTERNALS_LOADED = False # re-arm the (real) scan + assert "extadapter-real" in fresh_adapter_registry.known_adapter_kinds() + assert fresh_adapter_registry.external_adapter_errors() == {} + + +# --------------------------------------------------------------------------- # +# cli._make_adapters dispatch through the registry + + +def test_make_adapters_dispatches_registered_kind(fresh_adapter_registry, project, monkeypatch): + """A registered out-of-tree kind (the migrated Hermes case) dispatches with no + core branching: the synthesizing dev/review roles build the dev variant with + ``paths`` + the shared mux, triage builds the plain variant, and the + ``(cfg, synthesizes)`` cache shares the dev instance across dev+review.""" + registry = fresh_adapter_registry + registry.register_adapter("hermes", needs_mux=True, load=lambda: _stub_builder()) + monkeypatch.setattr(mux_mod, "_usable", lambda mux: True) + install_bmad_config(project) + _write_profile(project.project, "hermes", adapter="hermes", hookless=False) + _write_policy(project.project, '[adapter]\nname = "hermes"\n') + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + + adapters = cli._make_adapters(project.project, _run_dir(project.project), pol) + + assert isinstance(adapters["dev"], _StubDevAdapter) + assert adapters["dev"] is adapters["review"] # (cfg, synthesizes) sharing + assert adapters["dev"].paths.project == project.project + assert "mux" in adapters["dev"].kwargs # needs_mux=True threaded the transport + assert isinstance(adapters["triage"], _StubAdapter) + assert not isinstance(adapters["triage"], _StubDevAdapter) + assert adapters["triage"] is not adapters["dev"] + assert "mux" in adapters["triage"].kwargs + + +def test_make_adapters_skips_mux_for_needs_mux_false_kind( + fresh_adapter_registry, project, monkeypatch +): + """A ``needs_mux=False`` kind must never resolve the multiplexer — the same + guarantee the hookless HTTP adapter relies on.""" + registry = fresh_adapter_registry + registry.register_adapter("noxport", needs_mux=False, load=lambda: _stub_builder()) + + def no_mux(): + raise AssertionError("a needs_mux=False kind must not resolve a multiplexer") + + monkeypatch.setattr(mux_mod, "get_multiplexer", no_mux) + install_bmad_config(project) + _write_profile(project.project, "noxport", adapter="noxport") + _write_policy(project.project, '[adapter]\nname = "noxport"\n') + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + + adapters = cli._make_adapters(project.project, _run_dir(project.project), pol) + assert isinstance(adapters["dev"], _StubDevAdapter) + assert "mux" not in adapters["dev"].kwargs + + +def test_make_adapters_construct_error_becomes_systemexit( + fresh_adapter_registry, project, monkeypatch +): + """A family-declared construction failure raised in __init__ is converted to a + clean SystemExit — a run aborts with a message, not a traceback.""" + + class _Boom(Exception): + pass + + class _Exploding: + def __init__(self, **kwargs): + raise _Boom("server would not start") + + registry = fresh_adapter_registry + registry.register_adapter( + "boomer", + needs_mux=False, + load=lambda: AdapterBuilder(plain=_Exploding, dev=_Exploding, construct_error=(_Boom,)), + ) + install_bmad_config(project) + _write_profile(project.project, "boomer", adapter="boomer") + _write_policy(project.project, '[adapter]\nname = "boomer"\n') + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + + with pytest.raises(SystemExit, match="server would not start"): + cli._make_adapters(project.project, _run_dir(project.project), pol) + + +def test_make_adapters_unknown_kind_systemexit_names_profile(fresh_adapter_registry, project): + """A profile whose ``adapter`` names no registered kind aborts the run with a + SystemExit that names both the profile and the known kinds.""" + install_bmad_config(project) + _write_profile(project.project, "weird", adapter="ghostkind") + _write_policy(project.project, '[adapter]\nname = "weird"\n') + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + + with pytest.raises(SystemExit, match=r"weird.*ghostkind.*generic"): + cli._make_adapters(project.project, _run_dir(project.project), pol) + + +def test_make_adapters_generic_shares_synthesizing_but_not_triage( + fresh_adapter_registry, project, monkeypatch +): + """The ``(cfg, synthesizes)`` cache with the real builtin generic kind: dev and + review (same cfg, both bmad-dev-auto) share one GenericDevAdapter, triage on the + same cfg gets a separate plain GenericAdapter.""" + from bmad_loop.adapters.generic import GenericAdapter, GenericDevAdapter + + monkeypatch.setattr(mux_mod, "_usable", lambda mux: True) + install_bmad_config(project) + adapters = cli._make_adapters(project.project, _run_dir(project.project), policy_mod.load(None)) + assert isinstance(adapters["dev"], GenericDevAdapter) + assert adapters["dev"] is adapters["review"] + assert isinstance(adapters["triage"], GenericAdapter) + assert not isinstance(adapters["triage"], GenericDevAdapter) + assert adapters["triage"] is not adapters["dev"] + + +def test_adapters_command_lists_builtins_and_surfaces_failures( + scan_adapter_registry, capsys, tmp_path +): + """`bmad-loop adapters` renders the kind table and names a failed out-of-tree + package — the one place an operator looks when an installed adapter is missing.""" + import argparse + + registry, arm = scan_adapter_registry + + def boom(): + raise ImportError("No module named 'ghost_dependency'") + + arm(_FakeEntryPoint("brokenadapter", boom)) + args = argparse.Namespace(project=tmp_path) + assert cli.cmd_adapters(args) == 0 + captured = capsys.readouterr() + assert "generic" in captured.out # the table renders + assert "opencode-http" in captured.out + assert "brokenadapter" in captured.err + assert "ghost_dependency" in captured.err + + +def test_adapters_command_flags_dangling_kind_reference(fresh_adapter_registry, capsys, tmp_path): + """A project profile whose adapter kind never registered is named as a warning: + the table can't show a kind that isn't there, so the dangling reference is + surfaced explicitly.""" + import argparse + + _write_profile(tmp_path, "weird", adapter="ghostkind") + args = argparse.Namespace(project=tmp_path) + assert cli.cmd_adapters(args) == 0 + captured = capsys.readouterr() + assert "ghostkind" in captured.err + assert "weird" in captured.err + + +def test_make_adapters_opencode_http_dispatch_unchanged( + fresh_adapter_registry, project, monkeypatch +): + """Regression pin: routing the ``opencode-http`` profile through the registry + yields exactly the pre-registry adapters — OpencodeDevAdapter for the + synthesizing roles (never resolving a mux), OpencodeHttpAdapter for triage.""" + from bmad_loop.adapters import opencode_http + from bmad_loop.adapters.opencode_http import OpencodeDevAdapter, OpencodeHttpAdapter + + def no_mux(): + raise AssertionError("hookless opencode-http must not resolve a multiplexer") + + monkeypatch.setattr(opencode_http, "_require_httpx", lambda: object()) + monkeypatch.setattr(mux_mod, "get_multiplexer", no_mux) + install_bmad_config(project) + _write_policy(project.project, '[adapter]\nname = "opencode"\n') # alias → opencode-http + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + + adapters = cli._make_adapters(project.project, _run_dir(project.project), pol) + assert isinstance(adapters["dev"], OpencodeDevAdapter) + assert adapters["dev"] is adapters["review"] + assert adapters["dev"].profile.adapter == "opencode-http" + assert isinstance(adapters["triage"], OpencodeHttpAdapter) + assert not isinstance(adapters["triage"], OpencodeDevAdapter) diff --git a/tests/test_profile.py b/tests/test_profile.py index bbccd63f..30f01e1e 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -1,6 +1,9 @@ import pytest +from bmad_loop.adapters import profile as profile_mod from bmad_loop.adapters.profile import ( + CLIProfile, + HookSpec, ProfileError, get_profile, load_profiles, @@ -138,6 +141,155 @@ def test_unknown_profile_raises(): get_profile("acme-cli") +def test_adapter_field_defaults_to_generic_and_parses(): + """The adapter kind is read (not validated) at parse time: unset defaults to + the bundled tmux generic; opencode-http declares its HTTP adapter kind.""" + assert get_profile("claude").adapter == "generic" + assert get_profile("opencode-http").adapter == "opencode-http" + + +def test_adapter_field_not_validated_at_parse_time(tmp_path): + """A profile naming an unregistered adapter kind still PARSES — validity is + enforced later against the live registry (at construction / by `validate`), + never a hardcoded set here.""" + profiles_dir = tmp_path / ".bmad-loop" / "profiles" + profiles_dir.mkdir(parents=True) + (profiles_dir / "future.toml").write_text( + MINIMAL_PROFILE.replace("[hooks]", 'adapter = "not-a-real-kind-yet"\n[hooks]') + ) + assert load_profiles(tmp_path)["mycli"].adapter == "not-a-real-kind-yet" + + +# --------------------------------------------------------------------------- # +# bmad_loop.profiles entry-point discovery — the companion to the adapter +# registry so an out-of-tree package ships both its class and the selecting +# profile with zero project config. + + +@pytest.fixture +def profile_scan(monkeypatch): + """Isolate + re-arm the profile entry-point scan: snapshot/clear the module's + external-scan state, then hand back a hook to install fake entry points.""" + saved_loaded = profile_mod._EXTERNALS_LOADED + saved_profiles = dict(profile_mod._EXTERNAL_PROFILES) + saved_errors = dict(profile_mod._PROFILE_LOAD_ERRORS) + + def arm(*eps, scan_error=None): + def fake_entry_points(*, group): + assert group == profile_mod.PROFILES_GROUP + if scan_error is not None: + raise scan_error + return list(eps) + + monkeypatch.setattr(profile_mod.importlib.metadata, "entry_points", fake_entry_points) + profile_mod._EXTERNALS_LOADED = False + profile_mod._EXTERNAL_PROFILES.clear() + profile_mod._PROFILE_LOAD_ERRORS.clear() + + yield arm + + profile_mod._EXTERNALS_LOADED = saved_loaded + profile_mod._EXTERNAL_PROFILES.clear() + profile_mod._EXTERNAL_PROFILES.update(saved_profiles) + profile_mod._PROFILE_LOAD_ERRORS.clear() + profile_mod._PROFILE_LOAD_ERRORS.update(saved_errors) + + +class _FakeEntryPoint: + def __init__(self, name, load): + self.name = name + self._load = load + + def load(self): + return self._load() + + +def _plugin_profile(name="acme", adapter="acme"): + return CLIProfile(name=name, binary=name, adapter=adapter, hooks=HookSpec("none", "", {})) + + +def test_entry_point_profile_is_discovered(profile_scan): + """A pip-installed profile provider (a callable returning CLIProfiles) makes + its profile resolvable with no project TOML — the zero-config selection path.""" + profile_scan(_FakeEntryPoint("acme", lambda: (lambda: [_plugin_profile()]))) + prof = get_profile("acme") + assert prof.name == "acme" and prof.adapter == "acme" + assert profile_mod.external_profile_errors() == {} + + +def test_entry_point_profile_provider_may_be_iterable(profile_scan): + """The provider may be an iterable directly, not only a callable returning + one — both shapes are accepted.""" + profile_scan(_FakeEntryPoint("acme", lambda: [_plugin_profile()])) + assert "acme" in load_profiles() + + +def test_project_profile_overrides_entry_point(profile_scan, tmp_path): + """Precedence packaged < entry-point < project: a project-local TOML of the + same name wins over an entry-point profile.""" + profile_scan(_FakeEntryPoint("acme", lambda: [_plugin_profile(adapter="acme")])) + profiles_dir = tmp_path / ".bmad-loop" / "profiles" + profiles_dir.mkdir(parents=True) + (profiles_dir / "acme.toml").write_text( + MINIMAL_PROFILE.replace('name = "mycli"', 'name = "acme"') + ) + prof = load_profiles(tmp_path)["acme"] + assert prof.binary == "mycli" # the project TOML, not the entry-point profile + + +def test_entry_point_profile_can_override_packaged(profile_scan): + """Entry-point profiles overlay the packaged built-ins (packaged < + entry-point), so a plugin may re-point a bundled name.""" + profile_scan(_FakeEntryPoint("acme", lambda: [_plugin_profile(name="claude", adapter="acme")])) + assert load_profiles()["claude"].adapter == "acme" + + +def test_broken_profile_provider_degrades_and_is_recorded(profile_scan): + """A provider that blows up must not break profile loading: the built-ins + still load, and the failure is recorded for diagnostics.""" + + def boom(): + raise RuntimeError("half-installed plugin") + + profile_scan(_FakeEntryPoint("broken", boom)) + profiles = load_profiles() + assert "claude" in profiles # built-ins unaffected + assert list(profile_mod.external_profile_errors()) == ["broken"] + assert "half-installed" in profile_mod.external_profile_errors()["broken"] + + +def test_one_broken_profile_package_does_not_hide_the_rest(profile_scan): + """Per-entry isolation: a good provider still registers alongside a broken one.""" + + def boom(): + raise RuntimeError("broke") + + profile_scan( + _FakeEntryPoint("broken", boom), + _FakeEntryPoint("acme", lambda: [_plugin_profile()]), + ) + profiles = load_profiles() + assert "acme" in profiles + assert list(profile_mod.external_profile_errors()) == ["broken"] + + +def test_profile_provider_returning_junk_is_rejected(profile_scan): + """A provider that yields a non-CLIProfile is the package's bug — recorded, + never trusted into the profile map.""" + profile_scan(_FakeEntryPoint("acme", lambda: [object()])) + profiles = load_profiles() + assert "acme" not in profiles + assert "not CLIProfile" in profile_mod.external_profile_errors()["acme"] + + +def test_profile_scan_failure_degrades(profile_scan): + """The enumeration itself blowing up leaves built-in loading working, with the + scan failure recorded.""" + profile_scan(scan_error=RuntimeError("metadata index corrupt")) + assert "claude" in load_profiles() + assert "" in profile_mod.external_profile_errors() + + def test_render_prompt_passthrough_and_template(): claude = get_profile("claude") assert claude.render_prompt("/bmad-dev-auto 1-1-a") == "/bmad-dev-auto 1-1-a"