chore(server): apply prep-PR review feedback (P1 polish)

Code-reviewer feedback on PR #318 surfaced four items, all P1 polish
(no functional changes):

- _BROAD_SURFACE_BASES named set in serve.py replacing inline
  base.__name__ != "ADCPHandler" check. Future broad bases get added
  in one place.

- stacklevel=4 documented with the explicit call chain (warn →
  _warn_if_unregistered_subclass → _log_advertised_tools →
  _register_handler_tools → operator's serve()).

- 3 missing edge-case tests added:
  - test_init_subclass_three_level_chain_with_intermediate_declaration
  - test_init_subclass_forwards_kwargs_to_super (covers super()
    chain integrity in multi-inheritance — bystander base receives
    its __init_subclass__ kwargs through ADCPHandler's chain)
  - test_register_handler_tools_accepts_empty_iterable (pins the
    intended "zero AdCP tool surface" semantics)

Test count: 16 in test_register_handler_tools.py; 29 across the prep
PR test surface. Full suite: 2396 passed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: bokelley

src/adcp/server/serve.py

@@ -235,18 +235,29 @@ class introduces a new specialism (a custom subclass that's not in
     _warn_if_unregistered_subclass(handler, advertise_all=advertise_all)
 
 
+#: Bases whose tool set is broad-by-design — when an adopter subclass
+#: lands on one of these via MRO without registering its own
+#: ``advertised_tools``, the result is over-advertisement (the broad
+#: base's full set inherited unintentionally). Naming the rule rather
+#: than checking ``base.__name__ != "ADCPHandler"`` inline so future
+#: broad bases (a hypothetical ``UniversalHandler``) get added to one
+#: place — and a reviewer's first question becomes "is this base
+#: broad-by-design?" not "what's special about ADCPHandler?".
+_BROAD_SURFACE_BASES: frozenset[str] = frozenset({"ADCPHandler"})
+
+
 def _warn_if_unregistered_subclass(handler: ADCPHandler[Any], *, advertise_all: bool) -> None:
     """Emit a one-time ``UserWarning`` when a custom handler base bypasses
     the tool-discovery registry.
 
     The trigger: the concrete handler class itself isn't in
     ``_HANDLER_TOOLS``, has no ``advertised_tools`` declaration of its
-    own, and inherits its tool set from ``ADCPHandler`` (the broadest
-    base) rather than a specialized base like ``GovernanceHandler``.
-    That combination almost always means the adopter meant to declare a
-    focused tool set but forgot to register it; the framework
-    over-advertises by silently falling through to ADCPHandler's full
-    surface.
+    own, and inherits its tool set from a broad-surface base (see
+    :data:`_BROAD_SURFACE_BASES`) rather than a specialized base like
+    ``GovernanceHandler``. That combination almost always means the
+    adopter meant to declare a focused tool set but forgot to register
+    it; the framework over-advertises by silently falling through to
+    the broad base's full surface.
 
     Suppressed when ``advertise_all=True`` — that's the explicit "yes,
     advertise everything" opt-in.
@@ -260,15 +271,21 @@ def _warn_if_unregistered_subclass(handler: ADCPHandler[Any], *, advertise_all:
         # Should already have been auto-registered via __init_subclass__,
         # but defensively skip the warning if the attribute exists.
         return
-    # Walk MRO looking for a specialized SDK base (anything in the
-    # registry other than ADCPHandler itself). If one is found, the
-    # adopter is subclassing a focused base and inheriting its tool set
-    # — that's the documented pattern, no warning needed.
+    # Walk MRO looking for a specialized (non-broad-surface) SDK base.
+    # If one is found, the adopter is subclassing a focused base and
+    # inheriting its tool set — that's the documented pattern, no
+    # warning needed.
     has_specialized_parent = any(
-        base.__name__ in _HANDLER_TOOLS and base.__name__ != "ADCPHandler" for base in cls.__mro__
+        base.__name__ in _HANDLER_TOOLS and base.__name__ not in _BROAD_SURFACE_BASES
+        for base in cls.__mro__
     )
     if has_specialized_parent:
         return
+    # stacklevel=4 walks: warnings.warn → _warn_if_unregistered_subclass
+    # → _log_advertised_tools → _register_handler_tools → operator's
+    # serve()/create_mcp_server()/create_a2a_server() call site. If any
+    # of those frames are added or removed, recompute or refactor to
+    # use stacklevel=2 from a thin wrapper.
     warnings.warn(
         f"Handler class {cls.__name__!r} subclasses ADCPHandler directly "
         f"but isn't registered in the framework's tool-discovery "

tests/test_register_handler_tools.py

@@ -18,7 +18,7 @@
 from __future__ import annotations
 
 import warnings
-from typing import ClassVar
+from typing import Any, ClassVar
 
 import pytest
 
@@ -149,6 +149,65 @@ class _ChildBase(_ParentBase):
     assert "_ChildBase" not in _HANDLER_TOOLS
 
 
+def test_init_subclass_three_level_chain_with_intermediate_declaration(
+    cleanup_handler_registry,
+):
+    """Three-level inheritance where the *intermediate* base declares
+    ``advertised_tools`` registers AT the intermediate level. The leaf
+    inherits via MRO and isn't separately registered. Regression for
+    middle-of-the-chain registration."""
+
+    class _Root(ADCPHandler):
+        # No advertised_tools — root level.
+        pass
+
+    class _Intermediate(_Root):
+        # Declares its own — registers at this level.
+        advertised_tools: ClassVar[set[str]] = {"get_products", "create_media_buy"}
+
+    class _Leaf(_Intermediate):
+        # Inherits from intermediate; no own declaration.
+        pass
+
+    assert "_Root" not in _HANDLER_TOOLS
+    assert "_Intermediate" in _HANDLER_TOOLS
+    assert _HANDLER_TOOLS["_Intermediate"] == {"get_products", "create_media_buy"}
+    assert "_Leaf" not in _HANDLER_TOOLS
+
+
+def test_init_subclass_forwards_kwargs_to_super(cleanup_handler_registry):
+    """``__init_subclass__`` calls ``super().__init_subclass__(**kwargs)``
+    so PEP 487-style metaclass kwargs (e.g. from
+    ``__init_subclass__`` declared on a custom metaclass) flow through.
+    Without this, a future subclass using ``class X(ADCPHandler, mixin_kw=...)``
+    would lose the kwarg silently. Regression coverage by declaring a
+    bystander base that inspects ``cls`` on subclass creation."""
+    seen: list[type] = []
+
+    class _Bystander:
+        def __init_subclass__(cls, **kwargs: Any) -> None:
+            super().__init_subclass__(**kwargs)
+            seen.append(cls)
+
+    class _MultiInheritHandler(ADCPHandler, _Bystander):
+        advertised_tools: ClassVar[set[str]] = {"get_products"}
+
+    # Bystander's __init_subclass__ saw the new class — proves the
+    # super() chain is intact even with ADCPHandler.__init_subclass__
+    # in the chain. Without `super().__init_subclass__(**kwargs)`, the
+    # bystander would never fire.
+    assert _MultiInheritHandler in seen
+
+
+def test_register_handler_tools_accepts_empty_iterable(cleanup_handler_registry):
+    """An empty tool set is allowed — represents "this handler claims
+    zero tool surface." Registering it makes the handler-name known to
+    the registry without claiming any AdCP verbs. Useful for handlers
+    that exist purely for typed test-context plumbing."""
+    register_handler_tools("_EmptySetHandler", set())
+    assert _HANDLER_TOOLS["_EmptySetHandler"] == set()
+
+
 def test_init_subclass_idempotent_on_module_reload(cleanup_handler_registry):
     """Re-evaluating the same class body (test reload, ipython rerun)
     re-triggers ``__init_subclass__`` with the same tool set — must not