fix(adapters): registry residuals — builtin shadowing, collapsed load errors, adapter signature mismatch (#565, #566, #569)

CHANGELOG.md

@@ -272,6 +272,46 @@ breaking changes may land in a minor release.
   _implementing_ rate limiting prints all day — follow-up: #610. The four unseeded profiles
   (`codex`, `gemini`, `copilot`, `antigravity`) are unchanged and still ship none.
 
+- **An out-of-tree module that claims a bundled transport's name is no longer driven in the bundled
+  backend's place (#565).** Backend selection breaks ties on registration order, and the bundled
+  `tmux` / `psmux` entries were seeded only on the first multiplexer resolution — so any
+  registration that landed before it owned that name, and the whole process silently ran on a
+  third-party transport. The trigger is reachable rather than theoretical: a plugin's `[python]`
+  module is executed in-process by the plugin registry, which has no ordering relationship to when
+  the mux is first resolved. Registering a backend now seeds the bundled ones first, making
+  builtins-first an invariant of the registration itself rather than a property of which import ran
+  first — the same fix the adapter registry already carries. A shadowing external is still
+  registered, just behind the bundled entry.
+
+- **Two broken distributions that publish the same entry-point name no longer collapse to a single
+  recorded error (#566).** `importlib.metadata` does not deduplicate entry points across
+  distributions, and all three external-load scans — adapters, profiles and mux backends — recorded
+  failures by assignment into a name-keyed map, so the second failing package overwrote the first.
+  You fixed the one failure you were shown, then met the other on the next run with nothing saying
+  it had ever been there. Both reasons now surface, and every recorded external-load failure names
+  its **distribution** when one is resolvable — in `bmad-loop adapters`, `bmad-loop mux`,
+  `validate`'s human output and `validate --json` — because the entry-point name is not the
+  distribution you uninstall.
+
+  The limit, plainly: two failing same-named distributions still produce **one** finding and one
+  warning line, whose text now carries both reasons `"; "`-joined. The row count does not double.
+  That is what keeps the change contract-preserving — `detail`'s key set and value types are
+  unchanged, so `VALIDATE_SCHEMA_VERSION` stays 1.
+
+  **A deliberate scope addition:** the multiplexer's entry-point scan is now ordered by (name,
+  distribution), as the adapter and profile scans already were, so which of two same-named
+  distributions is recorded first is a fact about the packages rather than about `sys.path`.
+
+- **A run launch whose adapter class rejects a bootstrap keyword now aborts on a clean `error:`
+  line instead of a bare traceback (#569).** The construct call converted only the failures an
+  adapter family declares, and a signature mismatch is refused by the interpreter rather than by
+  the family — so an out-of-tree class whose `__init__` does not accept a keyword the bootstrap
+  passes escaped uncaught. The abort now names the profile, the adapter kind and the rejected
+  keyword; the exit code is unchanged at 1. Deliberately unchanged: a `TypeError` raised from
+  inside a working `__init__` still surfaces as itself, because that is a bug in that package
+  rather than a declared failure, and relabelling it as a signature mismatch would bury it. This is
+  message quality only — an error escaping here already unwound the composition it had started.
+
 ## [0.10.0] — 2026-08-14
 
 Much of this section is the `release/0.9.x` hotfix line brought forward onto `main` (#433).

src/bmad_loop/adapters/entrypoints.py

@@ -0,0 +1,63 @@
+"""Shared recording for the three ``bmad_loop.*`` entry-point scans.
+
+The adapter registry (:mod:`~.registry`), the profile loader (:mod:`~.profile`)
+and the multiplexer registry (:mod:`~.multiplexer`) each scan their own
+entry-point group, and each degrades a broken third-party distribution to a
+*recorded* reason rather than a crash. This module owns the one thing all three
+recordings must agree on: how a failure becomes an entry in that map.
+
+A leaf on purpose — standard library only, and **no import of a sibling
+adapters module**. Those three do not import each other today, and the package's
+builtins load lazily to keep that so; an edge from here into any of them would
+put a cycle one refactor away.
+
+Why the map stays keyed on the entry-point NAME. The key is what reaches
+``detail["entry_point"]`` in ``bmad-loop validate --json``. That document is
+schema-versioned and evolves additively (see ``documents.validate_document``,
+which contracts ``check`` as the matchable identity and states outright that
+``message``/``detail`` are for humans) — so widening the key to
+``(distribution, name)`` would change a value consumers can already see, while
+widening only the reason TEXT does not.
+
+Why reasons accumulate instead of overwriting.
+``importlib.metadata.entry_points(group=...)`` does not deduplicate across
+distributions: two installed packages may both advertise ``acme`` in one group,
+and the scan yields both. A plain assignment let the second failure silently
+overwrite the first, so an operator fixed one package and met the other on the
+next run with no sign it had ever been there. Reasons are joined with ``"; "``
+— reasons already contain colons, so a colon separator would be unreadable.
+
+Why the distribution labels each reason. The entry-point name is not the name
+you ``pip uninstall``, and two packages failing the same way otherwise render as
+the same sentence twice, with nothing to tell the operator there are two.
+
+The honest limit: a same-named collision still records ONE row, whose text now
+carries both reasons. The row count does not double — that is the price of
+leaving the key (and therefore ``--json``) untouched, and it still puts every
+reason in front of the operator.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+
+def record_load_error(errors: dict[str, str], ep: Any, exc: BaseException) -> None:
+    """Record ``exc`` against ``ep``'s name in ``errors``, appending to whatever a
+    same-named entry point already recorded.
+
+    ``ep`` is annotated ``Any`` deliberately: the callers pass a real
+    ``importlib.metadata.EntryPoint`` but every test passes a hand-rolled double,
+    so naming ``EntryPoint`` here would claim a contract this function does not
+    require (it touches ``.name`` and, defensively, ``.dist.name``).
+
+    The doubled ``getattr`` tolerates a double with no ``dist`` attribute at all
+    as well as a real entry point whose ``dist`` is ``None``; the truthiness
+    check then treats an empty distribution name as absent, the same
+    normalization the scans' ``or ""`` sort keys apply."""
+    dist = getattr(getattr(ep, "dist", None), "name", None)
+    reason = f"{type(exc).__name__}: {exc}"
+    if dist:
+        reason = f"{dist}: {reason}"
+    prior = errors.get(ep.name)
+    errors[ep.name] = f"{prior}; {reason}" if prior else reason

src/bmad_loop/adapters/multiplexer.py

@@ -12,10 +12,13 @@
 ``TerminalMultiplexer`` is the contract a backend author implements. Operation
 names mirror today's call sites verbatim so the migration is mechanical. Backends
 register themselves through :func:`register_multiplexer` (bundled ones from
-:func:`_load_builtin_backends`; out-of-tree ones at import time, triggered by the
-``bmad_loop.mux_backends`` entry-point scan in :func:`_load_external_backends` —
-so a pip/uv co-installed adapter package is selectable with no config step); the
-process-wide backend is selected by registry and returned by :func:`get_multiplexer`.
+:func:`_load_builtin_backends`, which :func:`register_multiplexer` seeds first so
+a bundled name keeps first-wins no matter who registers earliest; out-of-tree
+ones at import time — usually the ``bmad_loop.mux_backends`` entry-point scan in
+:func:`_load_external_backends`, so a pip/uv co-installed adapter package is
+selectable with no config step, but *any* import reaches it, a plugin's
+``[python]`` module included); the process-wide backend is selected by registry
+and returned by :func:`get_multiplexer`.
 
 Selection precedence (issue #87): the ``BMAD_LOOP_MUX_BACKEND`` env var, then the
 policy ``[mux] backend`` choice (installed once per CLI invocation via
@@ -37,6 +40,7 @@
 from pathlib import Path
 
 from .. import envvars
+from .entrypoints import record_load_error
 
 
 class MultiplexerError(Exception):
@@ -430,8 +434,24 @@ def register_multiplexer(
 ) -> None:
     """Register a transport backend. ``matches(sys.platform)`` decides automatic
     selection; ``name`` is the key for the ``BMAD_LOOP_MUX_BACKEND`` override.
-    Bundled backends register from :func:`_load_builtin_backends`; an out-of-tree
-    backend calls this at import time — no core edit required."""
+    Bundled backends register from :func:`_load_builtin_backends`, seeded here
+    rather than only by the resolution entry points, so an out-of-tree package can
+    never shadow a bundled name. An out-of-tree backend calls this at import time
+    — no core edit required.
+
+    Seeding on *this* side is what makes first-wins an invariant instead of an
+    ordering coincidence. ``_BACKENDS`` is an ordered list and every consumer
+    takes the first entry under a name (:func:`_factory_by_name` and all three
+    :func:`_select` loops), so whichever registration lands first owns the name.
+    An external module runs its ``register_multiplexer`` calls as an import side
+    effect, and that import is not always triggered by a mux resolution: a
+    plugin's ``[python]`` module is exec'd in-process by ``plugins/registry.py``,
+    which has no ordering relationship to the first :func:`get_multiplexer` call.
+    Arriving first, it would land ahead of the bundled tmux entry and be selected
+    in its place. Seeding keeps the bundled entry first; the external stays behind
+    it, since this list appends rather than dedups — which is exactly what a
+    shadowed name should look like."""
+    _load_builtin_backends()
     _BACKENDS.append((name, matches, factory))
     get_multiplexer.cache_clear()  # a later registration must not be shadowed by a cached pick
 
@@ -440,25 +460,36 @@ def _load_builtin_backends() -> None:
     """Register the bundled backends — tmux (POSIX) and psmux (native Windows);
     every other backend is out-of-tree and arrives via
     :func:`_load_external_backends` or a manual import. Idempotent and lazy
-    (called from :func:`get_multiplexer`, not at
-    module import) to stay cycle-safe. Registers inline rather than via
+    (called from :func:`get_multiplexer` and from :func:`register_multiplexer`,
+    not at module import) to stay cycle-safe. Registers inline rather than via
     tmux_backend's import side effect so the registry can be cleared and
     re-loaded deterministically (a re-import is a no-op once cached) —
-    mirroring ``process_host._load_builtin_hosts``."""
+    mirroring ``process_host._load_builtin_hosts``.
+
+    The flag sits between the imports and the registrations, and both halves of
+    that position are load-bearing: below the imports so a transient import
+    failure leaves the seeding retryable, above the registrations because they
+    re-enter this function through :func:`register_multiplexer`. The adapter twin
+    sets it at the very top only because its builtins are lazy thunks with
+    nothing to import first."""
     global _BUILTINS_LOADED
     if _BUILTINS_LOADED:
         return
     from .psmux_backend import PsmuxMultiplexer
     from .tmux_backend import TmuxMultiplexer
 
+    # Set after the imports but BEFORE the registrations. Below the imports so a
+    # transient import failure still retries; above the registrations because they
+    # re-enter this function through register_multiplexer, and a flag set
+    # afterwards would recurse without end.
+    _BUILTINS_LOADED = True
     # tmux is the default everywhere except native Windows (no tmux binary there);
     # get_multiplexer still falls back to tmux when no backend matches. Builtins
     # register before externals, so tmux keeps first-wins on any name collision.
     register_multiplexer("tmux", lambda platform: platform != "win32", TmuxMultiplexer)
     # psmux speaks the tmux CLI through its own distinctly-named binary, so
     # native Windows gets the tmux-family backend with a PowerShell dialect.
     register_multiplexer("psmux", lambda platform: platform == "win32", PsmuxMultiplexer)
-    _BUILTINS_LOADED = True  # set only after a successful import so a transient failure retries
 
 
 # The entry-point group an out-of-tree backend package advertises its module
@@ -480,26 +511,50 @@ def _load_external_backends() -> None:
     and the ``validate`` preflight via :func:`external_backend_errors`), not
     raised. Unlike ``_BUILTINS_LOADED``, the loaded-flag is set up front: a
     third-party import failure is not transient, and retrying on every
-    selection would re-import (and re-fail) each time."""
+    selection would re-import (and re-fail) each time.
+
+    Entry points are visited in (name, distribution) order. ``importlib.metadata``
+    yields them in distribution-discovery order, which varies with ``sys.path``, so
+    without an explicit sort two hosts carrying the same packages could register a
+    collision in a different order — and the order failures are recorded in would
+    be a fact about the install rather than about the packages.
+
+    The distribution belongs in the key because the name alone is NOT a total
+    order. ``entry_points(group=...)`` does not dedup across distributions, so two
+    packages advertising the same entry-point name come back as two entries, and
+    ``sorted`` is stable — a name-only key resolves that tie straight back into
+    ``sys.path`` order.
+
+    Such a same-named failure now ACCUMULATES rather than overwriting: recording
+    goes through :func:`~.entrypoints.record_load_error`, which appends under the
+    entry-point name and labels each reason with its distribution."""
     global _EXTERNALS_LOADED
     if _EXTERNALS_LOADED:
         return
     _EXTERNALS_LOADED = True
     try:
-        eps = importlib.metadata.entry_points(group=MUX_BACKENDS_GROUP)
+        eps = sorted(
+            importlib.metadata.entry_points(group=MUX_BACKENDS_GROUP),
+            key=lambda e: (e.name, getattr(e.dist, "name", "") or ""),
+        )
     except Exception as exc:  # diagnostics path, never crash selection
         _EXTERNAL_ERRORS["<entry-point scan>"] = f"{type(exc).__name__}: {exc}"
         return
     for ep in eps:
         try:
             ep.load()  # module import runs register_multiplexer(...)
         except Exception as exc:  # one bad package must not hide the rest
-            _EXTERNAL_ERRORS[ep.name] = f"{type(exc).__name__}: {exc}"
+            record_load_error(_EXTERNAL_ERRORS, ep, exc)
 
 
 def external_backend_errors() -> dict[str, str]:
-    """Entry-point name -> failure reason for every external backend that failed
-    to load this process (empty when all loaded). For diagnostics surfaces."""
+    """Entry-point name -> failure reason(s) for every external backend that failed
+    to load this process (empty when all loaded). For diagnostics surfaces.
+
+    One value may carry MORE than one reason, ``"; "``-joined: two distributions
+    may advertise the same entry-point name, and each of their failures is kept
+    (see :func:`~.entrypoints.record_load_error`). Each reason is labelled with
+    its distribution whenever one is resolvable."""
     return dict(_EXTERNAL_ERRORS)
 
 

src/bmad_loop/adapters/profile.py

@@ -16,7 +16,8 @@
 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.
+(:func:`external_profile_errors`), never a crash — one per failing distribution,
+kept even when two of them advertise the same name (:mod:`~.entrypoints`).
 
 Which adapter *class* drives a profile is the ``adapter`` field, resolved against
 the :mod:`~.registry` — it is read here but intentionally **not** checked against
@@ -40,6 +41,7 @@
 import regex
 
 from ..platform_util import has_parent_ref, is_absolute_path, names_tree_root
+from .entrypoints import record_load_error
 
 USAGE_PARSERS = {"claude-jsonl", "codex-rollout", "gemini-chat", "copilot-events", "none"}
 HOOK_DIALECTS = {
@@ -523,13 +525,19 @@ def _load_external_profiles() -> dict[str, CLIProfile]:
             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}"
+            record_load_error(_PROFILE_LOAD_ERRORS, ep, 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.
+    """Entry-point name -> failure reason(s) for every external profile provider
+    that failed to load this process (empty when all loaded). For diagnostics
+    surfaces.
+
+    One value may carry MORE than one reason, ``"; "``-joined: two distributions
+    may advertise the same entry-point name, and each of their failures is kept
+    (see :func:`~.entrypoints.record_load_error`). Each reason is labelled with
+    its distribution whenever one is resolvable.
 
     Performs the scan rather than assuming a neighbouring call already did. The
     only other trigger is :func:`load_profiles`, which ``validate`` reaches through

src/bmad_loop/adapters/registry.py

@@ -34,7 +34,8 @@
 are seeded by :func:`register_adapter` itself, so an external can never shadow a
 bundled name however early its import lands. A broken third-party distribution
 degrades to a recorded, surfaced reason (:func:`external_adapter_errors`) and can
-never break selection.
+never break selection — one reason per failing distribution, kept even when two of
+them advertise the same entry-point name (:mod:`~.entrypoints`).
 
 **Two deliberate asymmetries versus the multiplexer seam** (this is not a
 copy-paste omission):
@@ -58,6 +59,8 @@
 from collections.abc import Callable
 from dataclasses import dataclass
 
+from .entrypoints import record_load_error
+
 # The two bundled kind names, as constants rather than literals scattered across
 # modules. `validate`'s httpx check keys on OPENCODE_HTTP because httpx is *that
 # family's* optional extra — a fact about one bundled family, which is a different
@@ -241,13 +244,18 @@ def _load_external_adapters() -> None:
         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}"
+            record_load_error(_EXTERNAL_ERRORS, ep, exc)
 
 
 def external_adapter_errors() -> dict[str, str]:
-    """Entry-point name -> failure reason for every external adapter that failed
+    """Entry-point name -> failure reason(s) for every external adapter that failed
     to load this process (empty when all loaded). For diagnostics surfaces.
 
+    One value may carry MORE than one reason, ``"; "``-joined: two distributions
+    may advertise the same entry-point name, and each of their failures is kept
+    (see :func:`~.entrypoints.record_load_error`). Each reason is labelled with
+    its distribution whenever one is resolvable.
+
     Performs the scan itself rather than relying on a neighbouring
     ``known_adapter_kinds`` / ``detect_adapters`` call having run first — an
     accessor whose emptiness depends on call order reads as "nothing failed"."""