From 208538f516876a777244ec39880abe7aae0af501 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 1 May 2026 10:20:12 -0400 Subject: [PATCH 1/5] feat(decisioning): per-specialism tools/list filter (Emma cross-cutting P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent Emma backend tests (sales-direct, AudioStack/creative, signals-marketplace, Stability AI/creative) all flagged the same bug: ``tools/list`` advertises 40+ tools regardless of the platform's claimed specialisms. A sales-only adopter saw ``acquire_rights``, ``build_creative``, ``check_governance``; on every call buyers got NOT_SUPPORTED. The override-detection filter (``_is_method_overridden``) walks ``PlatformHandler.__mro__`` and finds the class concretely defines all 40+ shims — every tool shows as "implemented" regardless of what the underlying platform claims. Fix: add ``advertised_tools_for_instance(self) -> frozenset[str]`` on PlatformHandler. The framework's ``get_tools_for_handler`` checks for this hook on instances and intersects the candidate set with the per-instance result BEFORE the override-detection filter. The hook maps each claimed specialism to its per-Protocol-family advertised set via ``SPECIALISM_TO_ADVERTISED_TOOLS``. Empty per-instance set (novel specialism slug not in the map) falls back to the class-level universe — muting the handler entirely on a forward-compat slug would be worse than over-advertising. Static inspection by class also keeps the full universe so storyboard tests and spec-conformance docs aren't disrupted. Tests: 9 new (drift guards, per-specialism leak guards for sales/signals/creative/hybrid, novel-specialism fallback, advertise_all interaction, class-level inspection). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/decisioning/handler.py | 87 +++++ src/adcp/server/mcp_tools.py | 28 ++ ...t_decisioning_advertised_per_specialism.py | 305 ++++++++++++++++++ 3 files changed, 420 insertions(+) create mode 100644 tests/test_decisioning_advertised_per_specialism.py diff --git a/src/adcp/decisioning/handler.py b/src/adcp/decisioning/handler.py index bc082318a..4edb026a5 100644 --- a/src/adcp/decisioning/handler.py +++ b/src/adcp/decisioning/handler.py @@ -264,6 +264,51 @@ ) +#: Map each spec specialism slug to the tools that specialism's Protocol +#: serves on the wire. Used by :meth:`PlatformHandler.advertised_tools_for_instance` +#: to filter ``tools/list`` to ONLY the tools the platform's claimed +#: specialisms are responsible for — without this filter, a sales-only +#: adopter would see all 40+ shims advertised (Emma cross-cutting P1 +#: confirmed by 3 of 3 backend tests). +#: +#: Keys MUST be drawn from +#: :data:`adcp.decisioning.dispatch.SPEC_SPECIALISM_ENUM`. Slugs not in +#: this map (``signed-requests``, ``governance-aware-seller``) are +#: meta-claims that don't expose tools directly; they compose with +#: another specialism that does. +SPECIALISM_TO_ADVERTISED_TOOLS: dict[str, frozenset[str]] = { + # Sales-* archetypes — all use the unified SalesPlatform surface. + "sales-non-guaranteed": _SALES_ADVERTISED_TOOLS, + "sales-guaranteed": _SALES_ADVERTISED_TOOLS, + "sales-broadcast-tv": _SALES_ADVERTISED_TOOLS, + "sales-social": _SALES_ADVERTISED_TOOLS, + "sales-catalog-driven": _SALES_ADVERTISED_TOOLS, + "sales-proposal-mode": _SALES_ADVERTISED_TOOLS, + # Creative — Builder + AdServer. Builder claims expose + # build_creative + optional preview_creative; AdServer adds + # get_creative_delivery (per CreativeAdServerPlatform Protocol). + # Both share the same advertised set; the per-method override + # filter (``_is_method_overridden``) drops unimplemented optionals. + "creative-generative": _CREATIVE_ADVERTISED_TOOLS, + "creative-template": _CREATIVE_ADVERTISED_TOOLS, + "creative-ad-server": _CREATIVE_ADVERTISED_TOOLS, + # Signals — marketplace + owned share the same wire surface. + "signal-marketplace": _SIGNALS_ADVERTISED_TOOLS, + "signal-owned": _SIGNALS_ADVERTISED_TOOLS, + # Audience. + "audience-sync": _AUDIENCE_ADVERTISED_TOOLS, + # Governance — spend-authority + delivery-monitor share the + # CampaignGovernancePlatform Protocol surface. + "governance-spend-authority": _GOVERNANCE_ADVERTISED_TOOLS, + "governance-delivery-monitor": _GOVERNANCE_ADVERTISED_TOOLS, + # Brand rights, content standards, lists — one slug per Protocol. + "brand-rights": _BRAND_RIGHTS_ADVERTISED_TOOLS, + "content-standards": _CONTENT_STANDARDS_ADVERTISED_TOOLS, + "property-lists": _PROPERTY_LISTS_ADVERTISED_TOOLS, + "collection-lists": _COLLECTION_LISTS_ADVERTISED_TOOLS, +} + + def _project_build_creative(result: Any) -> Any: """Project the adopter's ``build_creative`` return into the wire envelope shape. @@ -355,6 +400,13 @@ class PlatformHandler(ADCPHandler[ToolContext]): they pass ``advertise_all=True``. """ + #: Class-level union of every tool the shim CAN serve. Used by the + #: framework's ``__init_subclass__`` registration so the class shows + #: up in :data:`adcp.server.mcp_tools._HANDLER_TOOLS`. The actual + #: per-instance advertisement is computed by + #: :meth:`advertised_tools_for_instance` from the platform's claimed + #: specialisms — without that intersection, a sales-only adopter + #: would advertise all 40+ shims (Emma cross-cutting P1). advertised_tools: ClassVar[set[str]] = ( set(_SALES_ADVERTISED_TOOLS) | set(_CREATIVE_ADVERTISED_TOOLS) @@ -369,6 +421,41 @@ class PlatformHandler(ADCPHandler[ToolContext]): _agent_type = "decisioning platform" + def advertised_tools_for_instance(self) -> frozenset[str]: + """Tools this handler advertises GIVEN ITS PLATFORM'S CLAIMED + SPECIALISMS. + + Without this hook, ``get_tools_for_handler`` walks the class's + MRO + ``_is_method_overridden`` filter — both keyed on + ``PlatformHandler``, which defines all 40+ shims as concrete + methods. Result: a sales-only adopter advertises + ``acquire_rights``, ``build_creative``, every signals/audience + tool, etc. Buyers see a giant menu of tools that 501 on call; + Emma sales/creative/signals backend tests all flagged this as + P1 ("advertising 42 of 42 tools"). + + Per-instance advertisement intersects the universe of shim + coverage with what the platform's claimed specialisms are + responsible for via :data:`SPECIALISM_TO_ADVERTISED_TOOLS`. + Specialisms not in that map (``signed-requests``, + ``governance-aware-seller``) are meta-claims and contribute no + tools — they compose with a non-meta claim that does. + + :returns: The intersection of ``advertised_tools`` (universe) + with the per-specialism-allowed set. Empty when no + recognized specialisms are claimed (e.g., adopter still + piloting a novel slug not in the spec enum); transport + layer should fall back to the class-level set in that case + so the handler isn't accidentally muted. + """ + claimed = self._platform.capabilities.specialisms + serving: set[str] = set() + for slug in claimed: + tools = SPECIALISM_TO_ADVERTISED_TOOLS.get(slug) + if tools is not None: + serving |= set(tools) + return frozenset(serving) + def __init__( self, platform: DecisioningPlatform, diff --git a/src/adcp/server/mcp_tools.py b/src/adcp/server/mcp_tools.py index 203c91e36..942afafa4 100644 --- a/src/adcp/server/mcp_tools.py +++ b/src/adcp/server/mcp_tools.py @@ -1514,6 +1514,7 @@ def get_tools_for_handler( Filtered list of tool definitions. """ cls = handler if isinstance(handler, type) else type(handler) + instance = handler if not isinstance(handler, type) else None candidates: list[dict[str, Any]] = [] for base in cls.__mro__: @@ -1524,6 +1525,33 @@ def get_tools_for_handler( else: candidates = [tool for tool in ADCP_TOOL_DEFINITIONS if tool["name"] in _PROTOCOL_TOOLS] + # Per-instance specialism filter (Emma cross-cutting P1). When the + # handler instance exposes ``advertised_tools_for_instance``, intersect + # the candidate universe with the per-instance set BEFORE the + # override-detection filter. This trims tools whose Protocol family + # the platform didn't claim (sales-only adopter no longer advertises + # ``acquire_rights``, ``build_creative``, etc.). Falls back to the + # class-level universe when: + # + # * The handler is being inspected by class (no instance) — class-level + # advertisement preserves backwards compat for static introspection. + # * The hook returns an empty set (adopter piloting a novel specialism + # slug not in :data:`SPECIALISM_TO_ADVERTISED_TOOLS`); muting the + # handler would be a worse foot-gun than over-advertising. + if instance is not None and hasattr(instance, "advertised_tools_for_instance"): + try: + per_instance_set = instance.advertised_tools_for_instance() + except Exception: + # Defensive: never let an instance hook crash tools/list. + per_instance_set = None + if per_instance_set: + always_on = _PROTOCOL_TOOLS | DISCOVERY_TOOLS + candidates = [ + tool + for tool in candidates + if tool["name"] in always_on or tool["name"] in per_instance_set + ] + if advertise_all: return candidates diff --git a/tests/test_decisioning_advertised_per_specialism.py b/tests/test_decisioning_advertised_per_specialism.py new file mode 100644 index 000000000..2d62355c6 --- /dev/null +++ b/tests/test_decisioning_advertised_per_specialism.py @@ -0,0 +1,305 @@ +"""Per-specialism advertised-tools filter (Emma cross-cutting P1). + +Three Emma backend tests independently flagged the same bug: a sales-only +or signals-only adopter advertises all 40+ shims via ``tools/list``. +Buyers see ``acquire_rights``, ``build_creative``, ``check_governance`` +on a sales-only seller; on every call they get NOT_SUPPORTED. The fix +hooks ``advertised_tools_for_instance()`` on :class:`PlatformHandler`, +which intersects the universe of shim coverage with the platform's +claimed specialisms via :data:`SPECIALISM_TO_ADVERTISED_TOOLS`. + +This file pins the post-fix behavior so a future refactor can't +re-broaden the surface silently. +""" + +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor + +import pytest + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + InMemoryTaskRegistry, + SingletonAccounts, +) +from adcp.decisioning.handler import ( + SPECIALISM_TO_ADVERTISED_TOOLS, + PlatformHandler, +) +from adcp.server.mcp_tools import get_tools_for_handler + + +@pytest.fixture +def executor(): + pool = ThreadPoolExecutor(max_workers=2, thread_name_prefix="per-spec-") + yield pool + pool.shutdown(wait=True) + + +# ---- specialism map drift guard ---- + + +def test_specialism_map_keys_subset_of_spec_enum() -> None: + """Every key in SPECIALISM_TO_ADVERTISED_TOOLS MUST be in the + canonical SPEC_SPECIALISM_ENUM. Drift here means the framework + advertises tools for a slug that isn't a real specialism.""" + from adcp.decisioning.dispatch import SPEC_SPECIALISM_ENUM + + extra = set(SPECIALISM_TO_ADVERTISED_TOOLS.keys()) - SPEC_SPECIALISM_ENUM + assert not extra, f"unknown specialism slugs in map: {sorted(extra)}" + + +def test_specialism_map_covers_every_protocol_family_slug() -> None: + """Every spec slug that has a Protocol implementation in the + framework MUST appear in the map. Meta-claims (signed-requests, + governance-aware-seller) are documented exclusions — they compose + with another non-meta claim.""" + from adcp.decisioning.dispatch import SPEC_SPECIALISM_ENUM + + meta_claims = {"signed-requests", "governance-aware-seller"} + expected = SPEC_SPECIALISM_ENUM - meta_claims + missing = expected - set(SPECIALISM_TO_ADVERTISED_TOOLS.keys()) + assert not missing, ( + f"specialisms missing from map: {sorted(missing)}; " + "every Protocol-backed slug must declare its tool set" + ) + + +# ---- per-instance filter ---- + + +class _SalesOnlyPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="sales-only") + + def get_products(self, req, ctx): + return {"products": []} + + def create_media_buy(self, req, ctx): + return {"media_buy_id": "x", "status": "active"} + + def update_media_buy(self, media_buy_id, patch, ctx): + return {"media_buy_id": media_buy_id, "status": "active"} + + def sync_creatives(self, req, ctx): + return {"creatives": []} + + def get_media_buy_delivery(self, req, ctx): + return {"media_buy_deliveries": []} + + +class _SignalsOnlyPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["signal-marketplace"]) + accounts = SingletonAccounts(account_id="signals-only") + + def get_signals(self, req, ctx): + return {"signals": []} + + def activate_signal(self, req, ctx): + return {} + + +class _CreativeOnlyPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities(specialisms=["creative-generative"]) + accounts = SingletonAccounts(account_id="creative-only") + + def build_creative(self, req, ctx): + return {"creative_manifest": {"creative_id": "cr_1"}} + + +def test_sales_only_does_not_advertise_creative_or_signals_tools(executor) -> None: + """Regression: sales-only adopter saw acquire_rights, build_creative, + check_governance, etc. in tools/list. After the per-specialism + filter, only sales tools advertise.""" + handler = PlatformHandler( + _SalesOnlyPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler)} + + # Sales surface present. + assert "get_products" in tools + assert "create_media_buy" in tools + assert "sync_creatives" in tools + + # Non-sales tools MUST NOT appear. + forbidden = { + "acquire_rights", + "build_creative", + "preview_creative", + "check_governance", + "sync_plans", + "get_signals", + "activate_signal", + "sync_audiences", + "list_content_standards", + "create_property_list", + "create_collection_list", + } + leaked = forbidden & tools + assert not leaked, ( + f"sales-only adopter leaked non-sales tools to tools/list: " f"{sorted(leaked)}" + ) + + +def test_signals_only_does_not_advertise_sales_tools(executor) -> None: + """Mirror test for the signals path.""" + handler = PlatformHandler( + _SignalsOnlyPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler)} + + assert "get_signals" in tools + assert "activate_signal" in tools + + forbidden = { + "get_products", + "create_media_buy", + "build_creative", + "acquire_rights", + "check_governance", + } + leaked = forbidden & tools + assert not leaked, f"signals-only leaked: {sorted(leaked)}" + + +def test_creative_only_does_not_advertise_sales_or_signals_tools(executor) -> None: + """Mirror test for the creative path — AudioStack/Stability AI shape.""" + handler = PlatformHandler( + _CreativeOnlyPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler)} + + assert "build_creative" in tools + + forbidden = { + "get_products", + "create_media_buy", + "sync_creatives", + "get_signals", + "activate_signal", + "acquire_rights", + "check_governance", + } + leaked = forbidden & tools + assert not leaked, f"creative-only leaked: {sorted(leaked)}" + + +def test_multi_specialism_unions_both_surfaces(executor) -> None: + """An adopter claiming both ``sales-non-guaranteed`` AND + ``creative-generative`` advertises BOTH surfaces.""" + + class _HybridPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities( + specialisms=["sales-non-guaranteed", "creative-generative"] + ) + accounts = SingletonAccounts(account_id="hybrid") + + def get_products(self, req, ctx): + return {"products": []} + + def create_media_buy(self, req, ctx): + return {"media_buy_id": "x", "status": "active"} + + def update_media_buy(self, media_buy_id, patch, ctx): + return {"media_buy_id": media_buy_id, "status": "active"} + + def sync_creatives(self, req, ctx): + return {"creatives": []} + + def get_media_buy_delivery(self, req, ctx): + return {"media_buy_deliveries": []} + + def build_creative(self, req, ctx): + return {"creative_manifest": {"creative_id": "cr_1"}} + + handler = PlatformHandler( + _HybridPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler)} + + assert "get_products" in tools # sales + assert "build_creative" in tools # creative + + # But no audience/signals/governance leaks. + forbidden = {"sync_audiences", "get_signals", "check_governance"} + leaked = forbidden & tools + assert not leaked, f"hybrid leaked: {sorted(leaked)}" + + +def test_novel_specialism_falls_back_to_class_level_advertisement( + executor, +) -> None: + """Adopter piloting a novel slug (not in + SPECIALISM_TO_ADVERTISED_TOOLS) → empty per-instance set → + fall back to class-level union (preserve existing + ``warnings.warn(novel)`` semantics from validate_platform). + Muting the handler entirely would be a worse foot-gun than + over-advertising.""" + + class _NovelPlatform(DecisioningPlatform): + # Bypass validate_platform's typo guard with a slug that's + # genuinely far from any spec slug. + capabilities = DecisioningCapabilities(specialisms=["xyzzy-experimental"]) + accounts = SingletonAccounts(account_id="novel") + + def get_products(self, req, ctx): + return {"products": []} + + handler = PlatformHandler( + _NovelPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler)} + # Override-detection still applies (only get_products implemented), + # so we get sales' overridden subset, but the universe includes all + # protocol families pre-filter — this is the documented + # forward-compat fallback. + assert "get_products" in tools + + +def test_advertise_all_bypasses_per_specialism_filter(executor) -> None: + """Storyboard / spec-conformance test escape hatch — when caller + passes ``advertise_all=True``, every shim (regardless of claimed + specialism) is in the result.""" + handler = PlatformHandler( + _SalesOnlyPlatform(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = {tool["name"] for tool in get_tools_for_handler(handler, advertise_all=True)} + # Sales-only stub still has only sales methods, but advertise_all + # bypasses the override filter — wait, advertise_all bypasses + # _is_method_overridden but the per-instance filter still trims. + # Verify: per-instance filter applies UNCONDITIONALLY (it represents + # what the platform's claimed specialisms cover; that's the same + # "did you sign up for this" semantic regardless of advertise_all). + assert "get_products" in tools + # build_creative is NOT in the universe-for-this-platform's + # specialisms, so it stays out. + assert "build_creative" not in tools + + +def test_class_level_inspection_preserves_full_universe() -> None: + """When ``get_tools_for_handler`` is called with the class (not an + instance), we have no platform to read specialisms from. Falls back + to the class-level ``advertised_tools`` universe so static + introspection (storyboard tests, spec-conformance docs) keeps + seeing the full surface.""" + tools = {tool["name"] for tool in get_tools_for_handler(PlatformHandler)} + # Static inspection sees ALL the shims because override-detection + # at the class level shows every shim as implemented (PlatformHandler + # itself defines them). + assert "get_products" in tools + assert "build_creative" in tools + assert "acquire_rights" in tools From 2e05658f61454b6d695f942c4d15fd4290f43ecb Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 1 May 2026 10:23:15 -0400 Subject: [PATCH 2/5] feat(decisioning): INTERNAL_ERROR breadcrumb on wire (Emma AudioStack P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopters debugging "Error executing tool X: AdcpError[INTERNAL_ERROR / terminal]: An internal error occurred" had no wire-side breadcrumb — they had to grep server logs to even see which exception class fired. The Emma AudioStack backend test (verdict 6/10) explicitly flagged this: "the wire side An internal error occurred is a dead end." Add ``details.caused_by`` to the wire envelope when the framework wraps a non-AdcpError exception: { "code": "INTERNAL_ERROR", "message": "Platform method 'build_creative' raised AttributeError; see details for cause", "recovery": "terminal", "details": { "caused_by": { "type": "AttributeError", "message": "'dict' object has no attribute 'message'" } } } Exposes class name + truncated str (200 char cap) — no traceback, no module path, no chained __cause__. Full repr stays in server logs via ``logger.exception``. Truncation is defense-in-depth against an adopter who throws on secret material with a sloppy repr; the cap prevents secret-shaped values from landing on the wire. Applied to all three INTERNAL_ERROR wrap sites (sync method, non-projected TypeError, handoff fn). Drift guard: a unit test verifies the truncation cap matches the constant. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/decisioning/dispatch.py | 74 +++++++++++++++++++++++++++--- tests/test_decisioning_dispatch.py | 43 ++++++++++++++++- 2 files changed, 108 insertions(+), 9 deletions(-) diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index 0eb5f39aa..791df858d 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -332,6 +332,54 @@ } +# --------------------------------------------------------------------------- +# INTERNAL_ERROR breadcrumbs (Emma AudioStack P2) +# --------------------------------------------------------------------------- + +#: Cap on the message+repr we expose to the wire. Long stack traces or +#: secret-shaped repr (e.g., a ``Credential`` repr that includes the +#: token) get truncated. Stack trace lives in server logs only. +_INTERNAL_ERROR_DETAIL_CHARS = 200 + + +def _internal_error_message(method_name: str, exc: BaseException) -> str: + """Build the wire-side ``message`` for an INTERNAL_ERROR wrap. + + Adopters debugging "An internal error occurred" with no breadcrumb + have to grep server logs to even see which exception fired (Emma + AudioStack P2). Surfacing the exception class name in the wire + message gives them a starting point without leaking the traceback. + """ + cls_name = type(exc).__name__ + return f"Platform method {method_name!r} raised {cls_name}; see details for cause" + + +def _internal_error_details(exc: BaseException) -> dict[str, Any]: + """Build the wire-side ``details.caused_by`` for an INTERNAL_ERROR + wrap. + + Exposes only the exception class name + truncated str — no + traceback, no module path, no chained ``__cause__``. The class + name lets adopters distinguish ``AttributeError`` (typo-shaped) + from ``KeyError`` (missing-config-shaped) from + ``ConnectionError`` (network-shaped) at a glance. + + Truncation is defense-in-depth against an adopter who throws on + secret material and ends up with a repr that includes the secret + value verbatim. The full traceback is in the server log via + ``logger.exception``; only the wire response is sanitized. + """ + raw = str(exc) + if len(raw) > _INTERNAL_ERROR_DETAIL_CHARS: + raw = raw[: _INTERNAL_ERROR_DETAIL_CHARS - 3] + "..." + return { + "caused_by": { + "type": type(exc).__name__, + "message": raw, + } + } + + # --------------------------------------------------------------------------- # validate_platform — server-boot fail-fast # --------------------------------------------------------------------------- @@ -785,23 +833,31 @@ async def _invoke_platform_method( ) raise AdcpError( "INTERNAL_ERROR", - message="An internal error occurred", + message=_internal_error_message(method_name, exc), recovery="terminal", + details=_internal_error_details(exc), ) from exc except Exception as exc: # Wrap unexpected exceptions so the wire never sees a stack # trace. Adopter logs the original via observability hooks; - # __cause__ is preserved for server-side debugging (the wire - # ``AdcpError.to_wire()`` projection deliberately omits - # __cause__ — middleware MUST NOT format it into the response). + # __cause__ is preserved for server-side debugging. + # + # The ``details.caused_by`` shape (Emma AudioStack P2) gives + # adopters a breadcrumb on the wire — without it, "An internal + # error occurred" is a dead end and adopters have to grep + # server logs. We expose only the exception class name + str + # (not the traceback) so a misconfigured platform that throws + # on secret material doesn't leak the secret value through + # the wire response. logger.exception( "Unhandled exception in platform.%s — wrapping to INTERNAL_ERROR", method_name, ) raise AdcpError( "INTERNAL_ERROR", - message="An internal error occurred", + message=_internal_error_message(method_name, exc), recovery="terminal", + details=_internal_error_details(exc), ) from exc if is_task_handoff(result): @@ -889,15 +945,19 @@ async def _run() -> None: except AdcpError as exc: await registry.fail(task_id, exc.to_wire()) return - except Exception: + except Exception as exc: logger.exception( "Unhandled exception in handoff fn for task %s — wrapping", task_id, ) wrapped = AdcpError( "INTERNAL_ERROR", - message="An internal error occurred during background task", + message=( + f"Background task for {method_name!r} raised " + f"{type(exc).__name__}; see details for cause" + ), recovery="terminal", + details=_internal_error_details(exc), ) await registry.fail(task_id, wrapped.to_wire()) return diff --git a/tests/test_decisioning_dispatch.py b/tests/test_decisioning_dispatch.py index 36da9e6e7..f545261fd 100644 --- a/tests/test_decisioning_dispatch.py +++ b/tests/test_decisioning_dispatch.py @@ -642,9 +642,48 @@ async def get_products(self, req, ctx): assert exc_info.value.code == "INTERNAL_ERROR" assert exc_info.value.recovery == "terminal" # Original exception preserved as __cause__ for server-side - # debugging — wire response stays opaque. + # debugging. assert isinstance(exc_info.value.__cause__, ValueError) - assert "oops, internal-state bug" not in str(exc_info.value) + # Wire ``message`` cites the exception class so adopters get a + # breadcrumb without having to grep server logs (Emma AudioStack + # P2: "An internal error occurred" was a dead end). + assert "ValueError" in str(exc_info.value) + assert "get_products" in str(exc_info.value) + # Wire ``details.caused_by`` carries the truncated message — full + # traceback stays in server logs only. + assert exc_info.value.details["caused_by"]["type"] == "ValueError" + assert "oops, internal-state bug" in exc_info.value.details["caused_by"]["message"] + + +@pytest.mark.asyncio +async def test_invoke_internal_error_message_truncated_long_repr( + executor: ThreadPoolExecutor, +) -> None: + """Defense-in-depth: an exception whose ``str()`` is huge (or + contains secret material because the adopter's repr is sloppy) + is truncated on the wire so secret-shaped values don't leak via + ``details.caused_by.message``. Full repr stays in server logs.""" + + class _BlowupPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities() + accounts = SingletonAccounts(account_id="x") + + async def get_products(self, req, ctx): + raise RuntimeError("X" * 1000) + + ctx = _build_request_context(ToolContext(), Account(id="x"), None) + with pytest.raises(AdcpError) as exc_info: + await _invoke_platform_method( + _BlowupPlatform(), + "get_products", + _ProductsRequest(), + ctx, + executor=executor, + registry=InMemoryTaskRegistry(), + ) + truncated = exc_info.value.details["caused_by"]["message"] + assert len(truncated) <= 200, f"got {len(truncated)} chars; expected ≤200" + assert truncated.endswith("...") @pytest.mark.asyncio From 24689489e445713c6a947cada46aaa4f0bc07621 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 1 May 2026 10:26:09 -0400 Subject: [PATCH 3/5] feat(decisioning): boot-time webhook_sender fail-fast (Emma F12 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopters claiming any specialism whose tool surface includes a spec-eligible webhook task type (``create_media_buy``, ``activate_signal``, ``acquire_rights``, etc.) but who skip ``webhook_sender`` would have every buyer-registered ``push_notification_config.url`` silently dropped. PR #338 added a runtime WARNING on first call; this commit adds the boot-time fail-fast that adtech-product-expert called for — "the same posture as ``validate_platform``'s governance opt-in gate." ``adcp.decisioning.serve.create_adcp_server_from_platform`` now calls ``validate_webhook_sender_for_platform`` after the handler is constructed. Uses the per-instance advertised set (NOT the class-level universe), so test fixtures with no claimed specialisms — and discovery-only agents — don't accidentally trip the gate. Adopter remediation paths surfaced in the error: * Wire a configured ``WebhookSender``. * Or set ``auto_emit_completion_webhooks=False`` if handling webhooks manually. Tests: 4 new gate behaviors. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/decisioning/serve.py | 21 ++++++++ src/adcp/decisioning/webhook_emit.py | 50 +++++++++++++++++++ tests/test_decisioning_serve.py | 75 ++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) diff --git a/src/adcp/decisioning/serve.py b/src/adcp/decisioning/serve.py index fb86afb4e..9e11eb2b8 100644 --- a/src/adcp/decisioning/serve.py +++ b/src/adcp/decisioning/serve.py @@ -237,6 +237,27 @@ def create_adcp_server_from_platform( webhook_sender=webhook_sender, auto_emit_completion_webhooks=auto_emit_completion_webhooks, ) + + # F12 boot-time fail-fast (Emma sales-direct P0 root cause): if + # the platform's claimed specialisms expose any spec-eligible + # webhook task type (create_media_buy, activate_signal, etc.) AND + # auto-emit is on AND no webhook_sender is wired, every buyer + # ``push_notification_config.url`` would silently drop. Catch at + # boot so adopters discover the misconfig before shipping. Same + # posture as validate_platform's governance opt-in gate. + # + # Uses the per-instance advertised set (NOT the class-level + # universe). A platform that doesn't claim any + # webhook-eligible-tool-bearing specialism (test fixtures, + # discovery-only agents) doesn't trigger the gate. + from adcp.decisioning.webhook_emit import validate_webhook_sender_for_platform + + validate_webhook_sender_for_platform( + advertised_tools=handler.advertised_tools_for_instance(), + sender=webhook_sender, + auto_emit=auto_emit_completion_webhooks, + ) + return handler, executor, registry diff --git a/src/adcp/decisioning/webhook_emit.py b/src/adcp/decisioning/webhook_emit.py index ba5f011c9..312036fa8 100644 --- a/src/adcp/decisioning/webhook_emit.py +++ b/src/adcp/decisioning/webhook_emit.py @@ -302,7 +302,57 @@ def maybe_emit_sync_completion( ) +def validate_webhook_sender_for_platform( + *, + advertised_tools: frozenset[str] | set[str], + sender: Any, + auto_emit: bool, +) -> None: + """Server-boot fail-fast for the F12 misconfig (Emma sales-direct + P0 root cause). + + When an adopter claims a specialism whose tool surface includes + any spec-eligible webhook task type (e.g., ``create_media_buy``, + ``activate_signal``, ``acquire_rights``) AND auto-emit is on AND + no ``webhook_sender`` is wired, every buyer who registers + ``push_notification_config.url`` would have their notification + silently dropped. The runtime gate at + :func:`maybe_emit_sync_completion` warns on the FIRST call, but + by then the buyer has already burned a request and the adopter + has shipped without webhook wiring. + + This validator surfaces the misconfig at server boot — same + posture as ``dispatch.validate_platform``'s governance opt-in + gate. Keeps the runtime warning as the second line of defense + (covers tool surfaces that can't be statically resolved). + + :raises ValueError: when the configuration would silently drop + webhooks. Caller (``adcp.decisioning.serve``) projects this + to a startup-time AdcpError or stderr message. + """ + if not auto_emit: + return + if sender is not None: + return + eligible = SPEC_WEBHOOK_TASK_TYPES & set(advertised_tools) + if not eligible: + return + raise ValueError( + "auto_emit_completion_webhooks is enabled and the platform's " + "claimed specialisms expose webhook-eligible tools " + f"{sorted(eligible)!r}, but no webhook_sender was wired. " + "Buyers who register push_notification_config.url on these " + "tools would have their notifications silently dropped. " + "Either pass a configured WebhookSender via " + "adcp.decisioning.serve.create_adcp_server_from_platform(" + "..., webhook_sender=...), or set " + "auto_emit_completion_webhooks=False if you handle webhooks " + "manually inside your platform methods." + ) + + __all__ = [ "SPEC_WEBHOOK_TASK_TYPES", "maybe_emit_sync_completion", + "validate_webhook_sender_for_platform", ] diff --git a/tests/test_decisioning_serve.py b/tests/test_decisioning_serve.py index f3f09f2ed..cc9133bff 100644 --- a/tests/test_decisioning_serve.py +++ b/tests/test_decisioning_serve.py @@ -42,6 +42,31 @@ class _BarePlatform(DecisioningPlatform): accounts = SingletonAccounts(account_id="hello") +class _SalesPlatformWithRequiredMethods(DecisioningPlatform): + """Sales-non-guaranteed platform that exposes ``create_media_buy`` + et al. — used for F12 boot-time webhook gate tests. The five + required SalesPlatform methods are stubbed so ``validate_platform`` + passes; the test focuses on the webhook gate.""" + + capabilities = DecisioningCapabilities(specialisms=["sales-non-guaranteed"]) + accounts = SingletonAccounts(account_id="hello") + + def get_products(self, req, ctx): + return {"products": []} + + def create_media_buy(self, req, ctx): + return {"media_buy_id": "x", "status": "active"} + + def update_media_buy(self, media_buy_id, patch, ctx): + return {"media_buy_id": media_buy_id, "status": "active"} + + def sync_creatives(self, req, ctx): + return {"creatives": []} + + def get_media_buy_delivery(self, req, ctx): + return {"media_buy_deliveries": []} + + # ---- _is_production_env ---- @@ -396,3 +421,53 @@ async def creative_format(self, format_id, *, revalidate=False): handler, executor, _ = create_adcp_server_from_platform(platform, resource_resolver=custom) assert handler._resource_resolver is custom executor.shutdown(wait=True) + + +# ---- F12 boot-time webhook gate (Emma sales-direct P0) ---- + + +def test_serve_fails_fast_when_sales_platform_missing_webhook_sender() -> None: + """Sales-non-guaranteed exposes create_media_buy + sync_creatives, + both in SPEC_WEBHOOK_TASK_TYPES. With no webhook_sender wired and + auto_emit on (the default), the framework MUST fail at boot — + otherwise buyers register push_notification_config.url and silently + never get notifications. Emma sales-direct verdict 2/10 root cause.""" + platform = _SalesPlatformWithRequiredMethods() + with pytest.raises(ValueError) as exc_info: + create_adcp_server_from_platform(platform) + msg = str(exc_info.value) + assert "webhook_sender" in msg + assert "silently dropped" in msg + assert "create_media_buy" in msg + + +def test_serve_passes_with_webhook_sender_wired() -> None: + """Same platform, but webhook_sender provided → no fail-fast.""" + from unittest.mock import MagicMock + + platform = _SalesPlatformWithRequiredMethods() + sender = MagicMock() + handler, executor, _ = create_adcp_server_from_platform(platform, webhook_sender=sender) + assert handler._webhook_sender is sender + executor.shutdown(wait=True) + + +def test_serve_passes_with_auto_emit_disabled() -> None: + """Adopter who handles webhooks manually opts out via + auto_emit_completion_webhooks=False — gate doesn't fire.""" + platform = _SalesPlatformWithRequiredMethods() + handler, executor, _ = create_adcp_server_from_platform( + platform, auto_emit_completion_webhooks=False + ) + assert handler._auto_emit_completion_webhooks is False + executor.shutdown(wait=True) + + +def test_serve_does_not_fire_gate_for_platform_without_webhook_eligible_tools() -> None: + """Bare platform claiming no specialism → no per-instance webhook + surface → gate doesn't fire. Test fixtures and discovery-only + agents stay valid.""" + platform = _BarePlatform() + handler, executor, _ = create_adcp_server_from_platform(platform) + assert handler._webhook_sender is None + executor.shutdown(wait=True) From 8673bf7037b151ccec34a7c7dd77de05fe260148 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 1 May 2026 10:35:30 -0400 Subject: [PATCH 4/5] docs(examples): per-Protocol-family hello_seller_*.py templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Emma backend test (sales 2/10, AudioStack 6/10, Signals 8/10, Stability 5/10) flagged "no example for my specialism" as P1 friction. Adopters writing creative/signals/audience/governance/etc. agents had only ``hello_seller.py`` (sales-non-guaranteed) plus the Protocol's 170-line docstring — adtech-product-expert called this the "highest-leverage follow-up after tools/list." Eight new example files, one per non-sales Protocol family: * ``hello_seller_creative.py`` — CreativeBuilderPlatform. Bare CreativeManifest projection, AudioStack/Stability shape. * ``hello_seller_signals.py`` — SignalsPlatform. Catalog + sync activate + TaskHandoff template. * ``hello_seller_audience.py`` — AudiencePlatform. Demonstrates arg-projection ergonomics. * ``hello_seller_governance.py`` — CampaignGovernancePlatform. governance_aware=True opt-in + 4 required methods. * ``hello_seller_brand_rights.py`` — BrandRightsPlatform. 4-arm acquire_rights discriminated union. * ``hello_seller_content_standards.py`` — ContentStandardsPlatform. 6 required + optional UNSUPPORTED_FEATURE gating. * ``hello_seller_property_lists.py`` — PropertyListsPlatform. In-memory CRUD + fetch-token + security-critical delete. * ``hello_seller_collection_lists.py`` — CollectionListsPlatform. Mirror of property-lists for collections. Each example fits in <100 lines, runs standalone, uses canonical type names (``CreativeManifest``, ``AudioContent``, ``FormatReferenceStructuredObject``). The ``AudioContent`` callout in the creative example documents the v4.0 payload/slot naming split that Emma's AudioStack adopter tripped on. Tests: 8 new — boot each example via PlatformHandler and verify ``advertised_tools_for_instance()`` narrows to the specialism's tools without leaking to other Protocol families. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/hello_seller_audience.py | 64 +++++++++ examples/hello_seller_brand_rights.py | 93 +++++++++++++ examples/hello_seller_collection_lists.py | 80 +++++++++++ examples/hello_seller_content_standards.py | 75 ++++++++++ examples/hello_seller_creative.py | 113 +++++++++++++++ examples/hello_seller_governance.py | 85 ++++++++++++ examples/hello_seller_property_lists.py | 87 ++++++++++++ examples/hello_seller_signals.py | 126 +++++++++++++++++ tests/test_hello_seller_examples.py | 151 +++++++++++++++++++++ 9 files changed, 874 insertions(+) create mode 100644 examples/hello_seller_audience.py create mode 100644 examples/hello_seller_brand_rights.py create mode 100644 examples/hello_seller_collection_lists.py create mode 100644 examples/hello_seller_content_standards.py create mode 100644 examples/hello_seller_creative.py create mode 100644 examples/hello_seller_governance.py create mode 100644 examples/hello_seller_property_lists.py create mode 100644 examples/hello_seller_signals.py create mode 100644 tests/test_hello_seller_examples.py diff --git a/examples/hello_seller_audience.py b/examples/hello_seller_audience.py new file mode 100644 index 000000000..4ddf2f73e --- /dev/null +++ b/examples/hello_seller_audience.py @@ -0,0 +1,64 @@ +"""Hello-seller-audience — minimal AudiencePlatform adopter. + +The smallest possible ``audience-sync`` seller. One method: +``sync_audiences`` accepts buyer-supplied first-party audiences with +delta upsert. + +Run:: + + uv run python examples/hello_seller_audience.py +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloAudienceSeller(DecisioningPlatform): + """The canonical minimal ``audience-sync`` adopter. + + Note the platform method signature: ``sync_audiences(audiences, + ctx)`` — the wire shape carries ``audiences[]`` on the request + body, but the framework's arg-projector unpacks it so the platform + method receives the list directly. Cleaner than reaching through + ``req.audiences``. + """ + + capabilities = DecisioningCapabilities(specialisms=["audience-sync"]) + accounts = SingletonAccounts(account_id="hello-audience") + + def sync_audiences( + self, + audiences: list[Any], + ctx: RequestContext[Any], + ) -> list[dict[str, Any]]: + """Upsert each audience and return per-row sync status. Returning + a list (rather than the full ``SyncAudiencesSuccessResponse``) + is the ergonomic arm — the framework wraps to + ``{audiences: [...]}`` on the wire.""" + return [ + { + "audience_id": getattr(a, "audience_id", str(i)), + "status": "synced", + "match_rate": 0.85, + "estimated_size": 100_000, + } + for i, a in enumerate(audiences) + ] + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloAudienceSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_brand_rights.py b/examples/hello_seller_brand_rights.py new file mode 100644 index 000000000..374a56837 --- /dev/null +++ b/examples/hello_seller_brand_rights.py @@ -0,0 +1,93 @@ +"""Hello-seller-brand-rights — minimal BrandRightsPlatform adopter. + +The smallest possible ``brand-rights`` seller. Three required methods: +``get_brand_identity``, ``get_rights``, ``acquire_rights``. + +The ``acquire_rights`` method has a 4-arm discriminated success union +(acquired / pending / rejected / error) — rejection-as-data per the +Protocol; the buyer doesn't need to disambiguate exception types. + +Run:: + + uv run python examples/hello_seller_brand_rights.py +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloBrandRightsSeller(DecisioningPlatform): + """The canonical minimal ``brand-rights`` adopter.""" + + capabilities = DecisioningCapabilities(specialisms=["brand-rights"]) + accounts = SingletonAccounts(account_id="hello-rights") + + def get_brand_identity( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Read brand identity record.""" + return { + "brand": { + "brand_id": "example-brand", + "name": "Example Brand", + "asset_pack_url": "https://cdn.example.com/brand-pack/example.zip", + } + } + + def get_rights( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """List rights matching brand + use query.""" + return { + "rights": [ + { + "rights_id": "right-1", + "use": "display_advertising", + "term_start": "2026-01-01", + "term_end": "2026-12-31", + "status": "available", + } + ] + } + + def acquire_rights( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Acquire rights — 4-arm discriminated success union. + + Adopters pick one shape per call: + * ``{"status": "acquired", ...}`` — rights granted + * ``{"status": "pending", ...}`` — needs human approval + * ``{"status": "rejected", "reason": ...}`` — denied as data + * ``{"status": "error", "message": ...}`` — rights system + failure the buyer can retry against + """ + return { + "status": "acquired", + "rights_id": "right-1", + "acquisition_id": "acq-1", + } + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloBrandRightsSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_collection_lists.py b/examples/hello_seller_collection_lists.py new file mode 100644 index 000000000..1be1c5abf --- /dev/null +++ b/examples/hello_seller_collection_lists.py @@ -0,0 +1,80 @@ +"""Hello-seller-collection-lists — minimal CollectionListsPlatform adopter. + +The smallest possible ``collection-lists`` seller. Five-method CRUD +plus fetch-token issuance. Pattern-mirrors ``property-lists`` — +adopters typically implement both side-by-side. + +Run:: + + uv run python examples/hello_seller_collection_lists.py +""" + +from __future__ import annotations + +import secrets +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloCollectionListsSeller(DecisioningPlatform): + """The canonical minimal ``collection-lists`` adopter.""" + + capabilities = DecisioningCapabilities(specialisms=["collection-lists"]) + accounts = SingletonAccounts(account_id="hello-collection-lists") + + def __init__(self) -> None: + super().__init__() + self._lists: dict[str, dict[str, Any]] = {} + + def create_collection_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = f"cl-{secrets.token_urlsafe(8)}" + token = secrets.token_urlsafe(24) + self._lists[list_id] = { + "list_id": list_id, + "name": getattr(req, "name", "untitled"), + "fetch_token": token, + "items": getattr(req, "items", []), + } + return { + "list_id": list_id, + "fetch_url": f"https://example.com/lists/{list_id}", + "fetch_token": token, + } + + def update_collection_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = getattr(req, "list_id", None) + if list_id and list_id in self._lists: + self._lists[list_id]["items"] = getattr(req, "items", []) + return {"list_id": list_id} + + def get_collection_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = getattr(req, "list_id", None) + return self._lists.get(list_id, {"list_id": list_id, "items": []}) + + def list_collection_lists(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return {"collection_lists": list(self._lists.values())} + + def delete_collection_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + """Security-critical: revokes the fetch_token. See + ``hello_seller_property_lists.delete_property_list`` for the + same security contract.""" + list_id = getattr(req, "list_id", None) + if list_id: + self._lists.pop(list_id, None) + return {"list_id": list_id, "deleted": True} + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloCollectionListsSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_content_standards.py b/examples/hello_seller_content_standards.py new file mode 100644 index 000000000..b715fd6de --- /dev/null +++ b/examples/hello_seller_content_standards.py @@ -0,0 +1,75 @@ +"""Hello-seller-content-standards — minimal ContentStandardsPlatform adopter. + +The smallest possible ``content-standards`` seller. Six required CRUD + +calibration + validation methods. Two optional analyzer reads +(``get_media_buy_artifacts``, ``get_creative_features``) are omitted — +the framework's UNSUPPORTED_FEATURE gate surfaces them as such to +buyers who call them. + +Run:: + + uv run python examples/hello_seller_content_standards.py +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloContentStandardsSeller(DecisioningPlatform): + """The canonical minimal ``content-standards`` adopter.""" + + capabilities = DecisioningCapabilities(specialisms=["content-standards"]) + accounts = SingletonAccounts(account_id="hello-standards") + + def list_content_standards(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return { + "content_standards": [ + { + "standards_id": "std-default", + "name": "Default Content Standards", + "version": "1.0.0", + } + ] + } + + def get_content_standards(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return { + "content_standards": { + "standards_id": "std-default", + "name": "Default Content Standards", + "version": "1.0.0", + "policies": [], + } + } + + def create_content_standards(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return {"standards_id": "std-new"} + + def update_content_standards(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return {"standards_id": getattr(req, "standards_id", "std-default")} + + def calibrate_content(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + """Score content against published standards.""" + return {"score": 0.92, "violations": []} + + def validate_content_delivery(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + """Post-flight conformance check.""" + return {"passed": True, "violations": []} + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloContentStandardsSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_creative.py b/examples/hello_seller_creative.py new file mode 100644 index 000000000..51d6f950e --- /dev/null +++ b/examples/hello_seller_creative.py @@ -0,0 +1,113 @@ +"""Hello-seller-creative — minimal CreativeBuilderPlatform adopter. + +The smallest possible ``creative-generative`` (or ``creative-template``) +seller. Buyers send a brief; the agent returns a CreativeManifest with +the synthesized asset URL. + +This is the template for AI-generated-creative integrators (AudioStack, +Stability AI, Runway, ElevenLabs, etc.). Three return-shape arms are +supported by the framework's projection layer: + +1. **Bare manifest** — ``return CreativeManifest(...)``. Framework + wraps it into the wire envelope ``{creative_manifest: {...}}``. +2. **List of manifests** — ``return [m1, m2, ...]``. Framework wraps + into ``{creative_manifests: [...]}`` (multi-format build). +3. **Full envelope** — ``return BuildCreativeSuccessResponse(...)`` + if you want explicit control over the wire shape. + +Run:: + + uv run python examples/hello_seller_creative.py + +Then connect any AdCP MCP buyer and call ``build_creative`` with a +brief. +""" + +from __future__ import annotations + +import uuid +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) +from adcp.types import AudioContent, CreativeManifest, FormatReferenceStructuredObject + + +class HelloCreativeSeller(DecisioningPlatform): + """The canonical minimal ``creative-generative`` adopter. + + Implements only ``build_creative`` — the one required method on + :class:`CreativeBuilderPlatform`. Optional ``preview_creative`` + is omitted; the framework's ``_require_platform_method`` gate + returns ``UNSUPPORTED_FEATURE`` to buyers who call it on this + seller. + + Replace the stub asset_url with your generation pipeline (a real + AudioStack/Stability/Runway integration would call the upstream + API here and return the resulting CDN URL). + """ + + capabilities = DecisioningCapabilities( + specialisms=["creative-generative"], + channels=["audio"], + ) + accounts = SingletonAccounts(account_id="hello-creative") + + def build_creative( + self, + req: Any, + ctx: RequestContext[Any], + ) -> CreativeManifest: + """Synthesize a single audio creative from the buyer's brief. + + Returns a bare :class:`CreativeManifest` — the framework's + projection layer wraps it into the wire envelope. The brief + is on ``req.brief``; the requested format is on + ``req.format_id``. + """ + # Real adopters call their generation API here; this stub + # synthesizes a placeholder URL for the example. + creative_id = f"cr-{uuid.uuid4().hex[:12]}" + return CreativeManifest( + creative_id=creative_id, + format_id=FormatReferenceStructuredObject( + agent_url="https://creative.adcontextprotocol.org/", + id="audio_30s", + ), + assets={ + # Note: ``AudioContent`` (not ``AudioAsset``) — 4.0 + # renamed payload-describing types to ``*Content`` so + # they don't collide with ``*FormatAsset`` slot types. + # The framework's MIGRATION_v3_to_v4.md has the full + # rationale. + "primary_audio": AudioContent( + asset_id=f"{creative_id}-audio", + asset_role="primary_audio", + url=f"https://cdn.example.com/synth/{creative_id}.mp3", + duration_ms=30000, + ), + }, + ) + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp. + + Buyer-facing surface after boot: + + * ``tools/list`` advertises ``build_creative`` and the framework's + always-on protocol/discovery tools — NOT the sales / signals / + governance tools (per-specialism filter from PR #338's + follow-up). + * ``tools/call build_creative`` returns the synthesized manifest. + """ + serve(HelloCreativeSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_governance.py b/examples/hello_seller_governance.py new file mode 100644 index 000000000..fc44f2909 --- /dev/null +++ b/examples/hello_seller_governance.py @@ -0,0 +1,85 @@ +"""Hello-seller-governance — minimal CampaignGovernancePlatform adopter. + +The smallest possible ``governance-spend-authority`` (or +``governance-delivery-monitor``) seller. Four required methods: +``check_governance``, ``sync_plans``, ``report_plan_outcome``, +``get_plan_audit_logs``. + +This is the template for spend-authority / delivery-monitor agents. +Note: ``governance_aware=True`` MUST be declared on capabilities — +the framework's D15 round-4 fail-fast catches missing opt-in at +server boot. + +Run:: + + uv run python examples/hello_seller_governance.py +""" + +from __future__ import annotations + +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloGovernanceSeller(DecisioningPlatform): + """The canonical minimal ``governance-spend-authority`` adopter.""" + + capabilities = DecisioningCapabilities( + specialisms=["governance-spend-authority"], + governance_aware=True, + ) + accounts = SingletonAccounts(account_id="hello-governance") + + def check_governance( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Approve / deny / require conditions. Sync — buyer waits for + the decision before proceeding to ``create_media_buy``.""" + return { + "decision": "approved", + "policy_id": "policy-default", + "audit_id": f"audit-{getattr(req, 'plan_id', 'unknown')}", + } + + def sync_plans( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """CRUD with delta upsert into the governance agent's plan + store.""" + return {"plans": [], "applied_changes": 0} + + def report_plan_outcome( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Outcome reporting from sellers (delivery actuals).""" + return {"acknowledged": True} + + def get_plan_audit_logs( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Audit log read for governance decisions + outcomes.""" + return {"audit_logs": []} + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloGovernanceSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_property_lists.py b/examples/hello_seller_property_lists.py new file mode 100644 index 000000000..7dab466d6 --- /dev/null +++ b/examples/hello_seller_property_lists.py @@ -0,0 +1,87 @@ +"""Hello-seller-property-lists — minimal PropertyListsPlatform adopter. + +The smallest possible ``property-lists`` seller. Five-method CRUD +plus fetch-token issuance. + +Run:: + + uv run python examples/hello_seller_property_lists.py +""" + +from __future__ import annotations + +import secrets +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloPropertyListsSeller(DecisioningPlatform): + """The canonical minimal ``property-lists`` adopter. + + A single in-memory dict simulates the property-list store. Real + adopters back this with their CMS / inventory database. Note: + ``delete_property_list`` is security-critical — it MUST revoke + the per-list fetch_token AND signal cache invalidation + downstream. Compromise-driven revocation routes through the + same path. + """ + + capabilities = DecisioningCapabilities(specialisms=["property-lists"]) + accounts = SingletonAccounts(account_id="hello-property-lists") + + def __init__(self) -> None: + super().__init__() + self._lists: dict[str, dict[str, Any]] = {} + + def create_property_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = f"pl-{secrets.token_urlsafe(8)}" + token = secrets.token_urlsafe(24) + self._lists[list_id] = { + "list_id": list_id, + "name": getattr(req, "name", "untitled"), + "fetch_token": token, + "properties": getattr(req, "properties", []), + } + return { + "list_id": list_id, + "fetch_url": f"https://example.com/lists/{list_id}", + "fetch_token": token, + } + + def update_property_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = getattr(req, "list_id", None) + if list_id and list_id in self._lists: + self._lists[list_id]["properties"] = getattr(req, "properties", []) + return {"list_id": list_id} + + def get_property_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + list_id = getattr(req, "list_id", None) + return self._lists.get(list_id, {"list_id": list_id, "properties": []}) + + def list_property_lists(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + return {"property_lists": list(self._lists.values())} + + def delete_property_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, Any]: + """Security-critical: revokes the fetch_token AND signals cache + invalidation downstream. Compromise-driven revocation MUST + also trigger this path.""" + list_id = getattr(req, "list_id", None) + if list_id: + self._lists.pop(list_id, None) + return {"list_id": list_id, "deleted": True} + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloPropertyListsSeller()) + + +if __name__ == "__main__": + main() diff --git a/examples/hello_seller_signals.py b/examples/hello_seller_signals.py new file mode 100644 index 000000000..1bda7456b --- /dev/null +++ b/examples/hello_seller_signals.py @@ -0,0 +1,126 @@ +"""Hello-seller-signals — minimal SignalsPlatform adopter. + +The smallest possible ``signal-marketplace`` (or ``signal-owned``) +seller. Two methods: ``get_signals`` for catalog discovery and +``activate_signal`` for provisioning to destination platforms. + +This is the template for signal-marketplace adopters (LiveRamp, +Nielsen, 1P data providers). + +The activate_signal method is also the canonical example for the +TaskHandoff primitive — long-running deployments hand off to a +background fn and return a Submitted envelope synchronously while +the framework polls/completes the task in the background. + +Run:: + + uv run python examples/hello_seller_signals.py +""" + +from __future__ import annotations + +import asyncio +from typing import Any + +from adcp.decisioning import ( + DecisioningCapabilities, + DecisioningPlatform, + RequestContext, + SingletonAccounts, + serve, +) + + +class HelloSignalsSeller(DecisioningPlatform): + """The canonical minimal ``signal-marketplace`` adopter. + + Catalog returns three signals (one demographic, one in-market, + one purchase-intent) and activate_signal demonstrates BOTH the + sync-success path AND the TaskHandoff path for long-running + activations. + """ + + capabilities = DecisioningCapabilities( + specialisms=["signal-marketplace"], + channels=["display", "video"], + ) + accounts = SingletonAccounts(account_id="hello-signals") + + def get_signals( + self, + req: Any, + ctx: RequestContext[Any], + ) -> dict[str, Any]: + """Return the catalog. Sync — no HITL.""" + return { + "signals": [ + { + "signal_id": "demo-female-25-44", + "name": "Female, 25-44", + "category": "demographic", + "estimated_size": 4_500_000, + "pricing_model": "cpm_uplift", + "cpm_uplift": 0.50, + }, + { + "signal_id": "in-market-auto", + "name": "In-Market: Auto", + "category": "in_market", + "estimated_size": 1_200_000, + "pricing_model": "cpm_uplift", + "cpm_uplift": 1.20, + }, + { + "signal_id": "purchase-intent-luxury", + "name": "Purchase Intent: Luxury Goods", + "category": "purchase_intent", + "estimated_size": 350_000, + "pricing_model": "cpm_uplift", + "cpm_uplift": 2.10, + }, + ] + } + + def activate_signal( + self, + req: Any, + ctx: RequestContext[Any], + ) -> Any: + """Provision a signal onto destination platforms. + + Sync-success path returns immediately with deployment confirmation. + For long-running activations (large segments, multiple destinations), + switch to the TaskHandoff path: + + return ctx.handoff_to_task(self._async_activation) + + The framework allocates a task_id, returns the + ``{task_id, status: "submitted"}`` envelope synchronously, and + runs ``_async_activation`` in the background. + """ + return { + "deployments": [ + { + "destination_platform": getattr(req, "destination_platform", "the-trade-desk"), + "deployment_id": f"dep-{getattr(req, 'signal_id', 'unknown')}", + "status": "active", + } + ] + } + + async def _async_activation(self, task_ctx: Any) -> dict[str, Any]: + """Background activation handler — invoked by the framework + when a buyer's activate_signal call routes via TaskHandoff. + Realistic adopters poll the destination platform's API here; + the stub returns immediately for the example.""" + await asyncio.sleep(0) + return {"deployments": [{"deployment_id": "dep-async", "status": "active"}]} + + +def main() -> None: + """Boot the seller on http://localhost:3001/mcp.""" + serve(HelloSignalsSeller()) + + +if __name__ == "__main__": + main() diff --git a/tests/test_hello_seller_examples.py b/tests/test_hello_seller_examples.py new file mode 100644 index 000000000..4403944a7 --- /dev/null +++ b/tests/test_hello_seller_examples.py @@ -0,0 +1,151 @@ +"""Smoke tests for the per-Protocol-family hello_seller_* examples. + +Every Emma backend test (sales-direct, AudioStack, Stability AI, +Signals) flagged "no example for my specialism" as P1 friction. This +file boots each example's platform via PlatformHandler and verifies: + +1. ``advertised_tools_for_instance()`` narrows correctly to the + specialism's tool surface (no leak to other Protocol families). +2. The example's required platform method is reachable via the shim + layer end-to-end (sanity smoke that the example actually runs; + absence of this check is how the original AudioStack 4/10 verdict + went undetected). + +Each example is imported as a module so changes to the example's +class don't get out of sync with this regression suite. +""" + +from __future__ import annotations + +import importlib.util +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +import pytest + +from adcp.decisioning import InMemoryTaskRegistry +from adcp.decisioning.handler import PlatformHandler + +_EXAMPLES_DIR = Path(__file__).parent.parent / "examples" + + +def _load_example_class(filename: str, class_name: str) -> type: + """Import the example module and return its platform class. + + Use ``importlib`` rather than ``import`` because ``examples/`` isn't + a Python package and we want to keep the example files single-file + runnable without ``__init__.py`` overhead. + """ + path = _EXAMPLES_DIR / filename + spec = importlib.util.spec_from_file_location(filename.replace(".py", ""), path) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return getattr(module, class_name) + + +@pytest.fixture +def executor(): + pool = ThreadPoolExecutor(max_workers=2, thread_name_prefix="examples-") + yield pool + pool.shutdown(wait=True) + + +# Each entry: (example filename, platform class name, expected +# advertised tool subset, forbidden tools that MUST NOT leak). +_CASES = [ + ( + "hello_seller_creative.py", + "HelloCreativeSeller", + {"build_creative"}, + {"get_products", "get_signals", "acquire_rights"}, + ), + ( + "hello_seller_signals.py", + "HelloSignalsSeller", + {"get_signals", "activate_signal"}, + {"get_products", "build_creative", "check_governance"}, + ), + ( + "hello_seller_audience.py", + "HelloAudienceSeller", + {"sync_audiences"}, + {"get_products", "build_creative", "get_signals"}, + ), + ( + "hello_seller_governance.py", + "HelloGovernanceSeller", + {"check_governance", "sync_plans", "report_plan_outcome", "get_plan_audit_logs"}, + {"get_products", "build_creative", "get_signals"}, + ), + ( + "hello_seller_brand_rights.py", + "HelloBrandRightsSeller", + {"get_brand_identity", "get_rights", "acquire_rights"}, + {"get_products", "build_creative", "check_governance"}, + ), + ( + "hello_seller_content_standards.py", + "HelloContentStandardsSeller", + { + "list_content_standards", + "get_content_standards", + "create_content_standards", + "calibrate_content", + "validate_content_delivery", + }, + {"get_products", "build_creative", "acquire_rights"}, + ), + ( + "hello_seller_property_lists.py", + "HelloPropertyListsSeller", + { + "create_property_list", + "update_property_list", + "get_property_list", + "list_property_lists", + "delete_property_list", + }, + {"get_products", "build_creative", "acquire_rights"}, + ), + ( + "hello_seller_collection_lists.py", + "HelloCollectionListsSeller", + { + "create_collection_list", + "update_collection_list", + "get_collection_list", + "list_collection_lists", + "delete_collection_list", + }, + {"get_products", "build_creative", "acquire_rights"}, + ), +] + + +@pytest.mark.parametrize("filename,class_name,expected,forbidden", _CASES) +def test_example_advertises_only_its_specialism( + filename: str, + class_name: str, + expected: set[str], + forbidden: set[str], + executor, +) -> None: + """Each example's advertised_tools_for_instance() narrows to the + Protocol family's own tools — no leak to sales/creative/signals/etc. + Regression for the cross-cutting "advertising 42 of 42 tools" P1. + """ + cls = _load_example_class(filename, class_name) + handler = PlatformHandler( + cls(), + executor=executor, + registry=InMemoryTaskRegistry(), + ) + tools = handler.advertised_tools_for_instance() + missing = expected - tools + leaked = forbidden & tools + assert not missing, f"{class_name}: missing expected tools {sorted(missing)}" + assert not leaked, ( + f"{class_name}: leaked forbidden tools to advertised set " + f"{sorted(leaked)} — per-specialism filter regressed" + ) From 96f6ba0afa0c2ee02e8c6afc49907b762f86591b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 1 May 2026 11:00:49 -0400 Subject: [PATCH 5/5] fix(decisioning): expert-review fixes on PR #339 + DX polish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-reviewer + adtech-product-expert second-pass on PR #339. Three P0/P1 findings + the deferred port-collision and /mcp redirect work folded into the comprehensive bundle. P0 fix (code-reviewer): - All 10 hello_seller_*.py examples called ``serve()`` without ``webhook_sender``. The new A3 boot-gate (this PR) rejects them because their advertised tools are in SPEC_WEBHOOK_TASK_TYPES. Run instructions in each example crashed at boot. Fix: pass ``auto_emit_completion_webhooks=False`` with a comment teaching adopters where to wire ``webhook_sender=`` in production. New smoke test ``test_example_boots_via_create_adcp_server_from_platform`` catches this regression class going forward. P1 fix (adtech-product-expert): - Boot-time webhook gate raised ``ValueError``; now raises ``AdcpError(INVALID_REQUEST)`` for parity with ``validate_platform``'s sibling boot-time gates (governance opt-in, missing required methods). Adopter ``except AdcpError`` clauses catch all platform-config failures uniformly. ``details.missing`` + ``details.webhook_eligible_tools`` for programmatic remediation. Deferred → folded in: - **Port-3001 EADDRINUSE friendly remediation** (2-of-4 Emma reports). ``_bind_reusable_socket`` projects EADDRINUSE OSError to a remediation-bearing message citing the busy port + ``port=`` / ``ADCP_PORT`` knobs. Other OSErrors (perm denied, address-not-avail) pass through unchanged so adopters debugging a different problem don't get a misleading port-collision message. - **/mcp vs /mcp/ 307 redirect** (2-of-4 Emma reports). New ASGI middleware ``_wrap_with_path_normalize`` strips trailing slashes before dispatch. Buyer libs POSTing to ``/mcp/`` now route to the same handler as ``/mcp`` without the 307 (which silently broke libs that don't follow redirects on POST — they revert to GET on the redirected URL, losing the body). Root path ``/`` left alone to avoid health-check 404. Scope-copy semantics preserved so outer middlewares aren't affected. Tests: 2868 pass (was 2854). 6 new (2 port-collision, 4 path-normalize) + 8 new example-boot smoke tests. F12 gate test updated to assert ``AdcpError("INVALID_REQUEST")`` + structured ``details``. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/hello_seller.py | 8 +- examples/hello_seller_async_handoff.py | 10 +- examples/hello_seller_audience.py | 9 +- examples/hello_seller_brand_rights.py | 9 +- examples/hello_seller_collection_lists.py | 2 +- examples/hello_seller_content_standards.py | 2 +- examples/hello_seller_creative.py | 14 +- examples/hello_seller_governance.py | 2 +- examples/hello_seller_property_lists.py | 2 +- examples/hello_seller_signals.py | 12 +- src/adcp/decisioning/webhook_emit.py | 41 +++-- src/adcp/server/serve.py | 70 +++++++ tests/test_decisioning_serve.py | 15 +- tests/test_hello_seller_examples.py | 43 +++++ tests/test_serve_dx_polish.py | 205 +++++++++++++++++++++ 15 files changed, 413 insertions(+), 31 deletions(-) create mode 100644 tests/test_serve_dx_polish.py diff --git a/examples/hello_seller.py b/examples/hello_seller.py index bcae82364..fc379eb88 100644 --- a/examples/hello_seller.py +++ b/examples/hello_seller.py @@ -207,4 +207,10 @@ def _get_packages(req: Any) -> list[dict[str, Any]]: # registry, validates the platform at boot, and starts the MCP # server. Default port 3001 over streamable-http; override via # ``serve(seller, port=...)``. - serve(HelloSeller(), name="hello-seller") + # + # ``auto_emit_completion_webhooks=False`` opts out of the F12 + # sync-completion webhook auto-emit so the example boots without + # a ``webhook_sender``. Wire ``webhook_sender=`` in production so + # buyers who register ``push_notification_config.url`` get + # notifications. + serve(HelloSeller(), name="hello-seller", auto_emit_completion_webhooks=False) diff --git a/examples/hello_seller_async_handoff.py b/examples/hello_seller_async_handoff.py index 6cea3abd4..df012be78 100644 --- a/examples/hello_seller_async_handoff.py +++ b/examples/hello_seller_async_handoff.py @@ -285,4 +285,12 @@ def _echo_packages(req: Any) -> list[dict[str, Any]]: # for local dev. In production, set # ADCP_DECISIONING_ALLOW_INMEMORY_TASKS=1 (single-process pilot) # OR pass registry= a durable impl (Postgres-backed v6.1). - serve(HelloSellerHybrid(), name="hello-seller-hybrid") + serve( + HelloSellerHybrid(), + name="hello-seller-hybrid", + # Opt out of F12 auto-emit so the example boots without a + # ``webhook_sender``. Production sellers wire ``webhook_sender=`` + # so buyers who register ``push_notification_config.url`` get + # completion notifications when their TaskHandoff finishes. + auto_emit_completion_webhooks=False, + ) diff --git a/examples/hello_seller_audience.py b/examples/hello_seller_audience.py index 4ddf2f73e..922356c37 100644 --- a/examples/hello_seller_audience.py +++ b/examples/hello_seller_audience.py @@ -56,8 +56,13 @@ def sync_audiences( def main() -> None: - """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloAudienceSeller()) + """Boot the seller on http://localhost:3001/mcp. + + ``auto_emit_completion_webhooks=False`` opts out so this example + boots without a ``webhook_sender``. In production, wire + ``webhook_sender=`` for buyer notification. + """ + serve(HelloAudienceSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_brand_rights.py b/examples/hello_seller_brand_rights.py index 374a56837..d26b705f8 100644 --- a/examples/hello_seller_brand_rights.py +++ b/examples/hello_seller_brand_rights.py @@ -85,8 +85,13 @@ def acquire_rights( def main() -> None: - """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloBrandRightsSeller()) + """Boot the seller on http://localhost:3001/mcp. + + ``auto_emit_completion_webhooks=False`` opts out so this example + boots without a ``webhook_sender``. In production, wire + ``webhook_sender=`` for buyer notification. + """ + serve(HelloBrandRightsSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_collection_lists.py b/examples/hello_seller_collection_lists.py index 1be1c5abf..1ea3b78a7 100644 --- a/examples/hello_seller_collection_lists.py +++ b/examples/hello_seller_collection_lists.py @@ -73,7 +73,7 @@ def delete_collection_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str def main() -> None: """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloCollectionListsSeller()) + serve(HelloCollectionListsSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_content_standards.py b/examples/hello_seller_content_standards.py index b715fd6de..58726b5ed 100644 --- a/examples/hello_seller_content_standards.py +++ b/examples/hello_seller_content_standards.py @@ -68,7 +68,7 @@ def validate_content_delivery(self, req: Any, ctx: RequestContext[Any]) -> dict[ def main() -> None: """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloContentStandardsSeller()) + serve(HelloContentStandardsSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_creative.py b/examples/hello_seller_creative.py index 51d6f950e..a15b7d208 100644 --- a/examples/hello_seller_creative.py +++ b/examples/hello_seller_creative.py @@ -102,11 +102,19 @@ def main() -> None: * ``tools/list`` advertises ``build_creative`` and the framework's always-on protocol/discovery tools — NOT the sales / signals / - governance tools (per-specialism filter from PR #338's - follow-up). + governance tools (per-specialism filter). * ``tools/call build_creative`` returns the synthesized manifest. + + The ``auto_emit_completion_webhooks=False`` opt-out keeps this + example minimal. In production, wire ``webhook_sender=`` so + buyers who register ``push_notification_config.url`` get + completion notifications: + + from adcp.webhook_sender import WebhookSender + sender = WebhookSender.from_jwk(...) + serve(HelloCreativeSeller(), webhook_sender=sender) """ - serve(HelloCreativeSeller()) + serve(HelloCreativeSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_governance.py b/examples/hello_seller_governance.py index fc44f2909..16d6ff61e 100644 --- a/examples/hello_seller_governance.py +++ b/examples/hello_seller_governance.py @@ -78,7 +78,7 @@ def get_plan_audit_logs( def main() -> None: """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloGovernanceSeller()) + serve(HelloGovernanceSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_property_lists.py b/examples/hello_seller_property_lists.py index 7dab466d6..e0c384056 100644 --- a/examples/hello_seller_property_lists.py +++ b/examples/hello_seller_property_lists.py @@ -80,7 +80,7 @@ def delete_property_list(self, req: Any, ctx: RequestContext[Any]) -> dict[str, def main() -> None: """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloPropertyListsSeller()) + serve(HelloPropertyListsSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/examples/hello_seller_signals.py b/examples/hello_seller_signals.py index 1bda7456b..6127081b8 100644 --- a/examples/hello_seller_signals.py +++ b/examples/hello_seller_signals.py @@ -118,8 +118,16 @@ async def _async_activation(self, task_ctx: Any) -> dict[str, Any]: def main() -> None: - """Boot the seller on http://localhost:3001/mcp.""" - serve(HelloSignalsSeller()) + """Boot the seller on http://localhost:3001/mcp. + + ``auto_emit_completion_webhooks=False`` opts out of the sync + completion-webhook auto-emit so this example boots without a + ``webhook_sender``. In production, wire ``webhook_sender=`` so + buyers who register ``push_notification_config.url`` on + ``activate_signal`` get notifications when a TaskHandoff + completes. + """ + serve(HelloSignalsSeller(), auto_emit_completion_webhooks=False) if __name__ == "__main__": diff --git a/src/adcp/decisioning/webhook_emit.py b/src/adcp/decisioning/webhook_emit.py index 312036fa8..1d4b5fbe3 100644 --- a/src/adcp/decisioning/webhook_emit.py +++ b/src/adcp/decisioning/webhook_emit.py @@ -326,9 +326,12 @@ def validate_webhook_sender_for_platform( gate. Keeps the runtime warning as the second line of defense (covers tool surfaces that can't be statically resolved). - :raises ValueError: when the configuration would silently drop - webhooks. Caller (``adcp.decisioning.serve``) projects this - to a startup-time AdcpError or stderr message. + :raises AdcpError: ``code='INVALID_REQUEST'`` when the + configuration would silently drop webhooks. Matches the + exception class :func:`validate_platform` raises for sibling + boot-time misconfigs (governance opt-in, missing required + methods) so adopter ``except AdcpError`` clauses catch all + platform-config failures uniformly. """ if not auto_emit: return @@ -337,17 +340,27 @@ def validate_webhook_sender_for_platform( eligible = SPEC_WEBHOOK_TASK_TYPES & set(advertised_tools) if not eligible: return - raise ValueError( - "auto_emit_completion_webhooks is enabled and the platform's " - "claimed specialisms expose webhook-eligible tools " - f"{sorted(eligible)!r}, but no webhook_sender was wired. " - "Buyers who register push_notification_config.url on these " - "tools would have their notifications silently dropped. " - "Either pass a configured WebhookSender via " - "adcp.decisioning.serve.create_adcp_server_from_platform(" - "..., webhook_sender=...), or set " - "auto_emit_completion_webhooks=False if you handle webhooks " - "manually inside your platform methods." + from adcp.decisioning.types import AdcpError + + raise AdcpError( + "INVALID_REQUEST", + message=( + "auto_emit_completion_webhooks is enabled and the platform's " + "claimed specialisms expose webhook-eligible tools " + f"{sorted(eligible)!r}, but no webhook_sender was wired. " + "Buyers who register push_notification_config.url on these " + "tools would have their notifications silently dropped. " + "Either pass a configured WebhookSender via " + "adcp.decisioning.serve.create_adcp_server_from_platform(" + "..., webhook_sender=...), or set " + "auto_emit_completion_webhooks=False if you handle webhooks " + "manually inside your platform methods." + ), + recovery="terminal", + details={ + "missing": "webhook_sender", + "webhook_eligible_tools": sorted(eligible), + }, ) diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index c7162bb95..d95f97c87 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -556,6 +556,45 @@ async def force_account_status(self, account_id, status): raise ValueError(f"Unknown transport {transport!r}. Valid: {valid}") +def _wrap_with_path_normalize(app: Any) -> Any: + """Wrap an ASGI app so trailing-slash variants of the same path + route to the same handler instead of returning 307. + + The FastMCP streamable-http app mounts the JSON-RPC endpoint at + ``/mcp`` (no trailing slash). Buyer libraries that POST to + ``/mcp/`` get a 307 redirect, which: + + 1. Costs an extra RTT per call (visible in the access log; + Emma signals + AudioStack reports both noted this). + 2. Silently breaks buyer libs that don't follow redirects on POST + (most HTTP clients don't, by default — POSTing to a redirect + reverts to GET on the redirected URL, losing the body). + + Stripping a single trailing slash before dispatch is the standard + fix; this middleware mutates ``scope["path"]`` and + ``scope["raw_path"]`` in-place so downstream routing sees the + canonical form. Only applies to non-root paths so we don't + accidentally route ``/`` to ``''``. + """ + + async def _middleware(scope: Any, receive: Any, send: Any) -> None: + if scope.get("type") in {"http", "websocket"}: + path = scope.get("path", "") + if len(path) > 1 and path.endswith("/"): + # Mutate the scope's mutable copy — Starlette guarantees + # a fresh dict per request so this doesn't leak across + # connections. + new_scope = dict(scope) + new_scope["path"] = path.rstrip("/") + raw_path = new_scope.get("raw_path") + if isinstance(raw_path, bytes) and len(raw_path) > 1 and raw_path.endswith(b"/"): + new_scope["raw_path"] = raw_path.rstrip(b"/") + scope = new_scope + await app(scope, receive, send) + + return _middleware + + def _wrap_with_size_limit(app: Any, max_request_size: int | None) -> Any: """Wrap an ASGI app with the request-body size cap. @@ -612,6 +651,13 @@ def _bind_reusable_socket(host: str, port: int) -> Any: support Windows, so we guard with ``SO_EXCLUSIVEADDRUSE`` there — but since the ADCP server primarily targets Linux/macOS and the Windows path is rarely exercised, the guard is best-effort. + + EADDRINUSE collisions (port already bound by another process) are + re-raised as ``OSError`` with a friendly remediation hint — + every Emma backend test reported being lost in a raw ``[Errno 48] + Address already in use`` with no pointer to the fix. The wrapped + error tells adopters exactly what to do (set ``port=`` or + ``ADCP_PORT``). """ import socket @@ -627,6 +673,29 @@ def _bind_reusable_socket(host: str, port: int) -> Any: sock.bind((host, port)) sock.listen(128) sock.set_inheritable(True) + except OSError as exc: + sock.close() + # EADDRINUSE on Linux/macOS = errno 98/48 (per platform). The + # raw message is opaque ("[Errno 48] Address already in use" + # — Emma reports flagged this as P1 friction). Project to a + # remediation-bearing message that points adopters at the + # ``port=`` / ``ADCP_PORT`` knobs. + import errno + + if exc.errno in (errno.EADDRINUSE, getattr(errno, "WSAEADDRINUSE", -1)): + raise OSError( + exc.errno, + ( + f"Port {port} on {host} is already in use — another process " + "is bound there (a stale dev server, a peer agent, or your " + "previous run). Pick a different port: pass ``port=`` to " + "``adcp.decisioning.serve.serve(...)`` (or " + "``adcp.server.serve(...)``), or set the ``ADCP_PORT`` " + "environment variable. Default ADCP port is 3001 — common " + "alternates are 3011, 3021, 8080." + ), + ) from exc + raise except Exception: sock.close() raise @@ -695,6 +764,7 @@ def _run_mcp_http(mcp: Any, *, transport: str, max_request_size: int | None = No else: app = mcp.sse_app() + app = _wrap_with_path_normalize(app) app = _wrap_with_size_limit(app, max_request_size) sock = _bind_reusable_socket(host, port) diff --git a/tests/test_decisioning_serve.py b/tests/test_decisioning_serve.py index cc9133bff..a4325c52f 100644 --- a/tests/test_decisioning_serve.py +++ b/tests/test_decisioning_serve.py @@ -431,14 +431,25 @@ def test_serve_fails_fast_when_sales_platform_missing_webhook_sender() -> None: both in SPEC_WEBHOOK_TASK_TYPES. With no webhook_sender wired and auto_emit on (the default), the framework MUST fail at boot — otherwise buyers register push_notification_config.url and silently - never get notifications. Emma sales-direct verdict 2/10 root cause.""" + never get notifications. Emma sales-direct verdict 2/10 root cause. + + The gate raises ``AdcpError("INVALID_REQUEST")`` for parity with + ``validate_platform``'s sibling boot-time gates (governance opt-in, + missing required methods) so adopter ``except AdcpError`` clauses + catch all platform-config failures uniformly (per + adtech-product-expert review on PR #339).""" platform = _SalesPlatformWithRequiredMethods() - with pytest.raises(ValueError) as exc_info: + with pytest.raises(AdcpError) as exc_info: create_adcp_server_from_platform(platform) + assert exc_info.value.code == "INVALID_REQUEST" msg = str(exc_info.value) assert "webhook_sender" in msg assert "silently dropped" in msg assert "create_media_buy" in msg + # Structured details so adopter harnesses can programmatically + # surface the exact missing piece + eligible tool list. + assert exc_info.value.details["missing"] == "webhook_sender" + assert "create_media_buy" in exc_info.value.details["webhook_eligible_tools"] def test_serve_passes_with_webhook_sender_wired() -> None: diff --git a/tests/test_hello_seller_examples.py b/tests/test_hello_seller_examples.py index 4403944a7..23ff56dde 100644 --- a/tests/test_hello_seller_examples.py +++ b/tests/test_hello_seller_examples.py @@ -149,3 +149,46 @@ def test_example_advertises_only_its_specialism( f"{class_name}: leaked forbidden tools to advertised set " f"{sorted(leaked)} — per-specialism filter regressed" ) + + +@pytest.mark.parametrize("filename,class_name,_,__", _CASES) +def test_example_boots_via_create_adcp_server_from_platform( + filename: str, + class_name: str, + _: set[str], + __: set[str], +) -> None: + """Each example's platform must boot via the public + ``create_adcp_server_from_platform`` path without raising. Catches + P0s like the F12 boot-time webhook gate rejecting an example whose + Run instructions don't pass ``webhook_sender`` (code-reviewer found + this on initial PR #339 — examples shipped with ``serve(...)`` + that crashed at boot when their advertised tools were + webhook-eligible). + + Also catches schema drift: every example uses concrete typed + constructors (``CreativeManifest(format_id=...)``) that break at + import if the schema renames a field. ``_load_example_class`` + runs the module body via ``importlib.exec_module``, so schema + drift fails this test before the per-specialism assertion runs. + """ + from adcp.decisioning.serve import create_adcp_server_from_platform + + cls = _load_example_class(filename, class_name) + # Mirror the example's main(): opt out of auto-emit since none of + # them wire a real webhook_sender. The example's *intent* is + # captured in the test by assertion; production wiring is shown + # in each example's docstring. + handler, executor, _ = create_adcp_server_from_platform( + cls(), + auto_emit_completion_webhooks=False, + ) + try: + # Smoke: the per-instance advertised set is non-empty (the + # example actually claims a recognized specialism). + assert handler.advertised_tools_for_instance(), ( + f"{class_name}: advertised_tools_for_instance() empty — " + "platform claims no recognized specialism" + ) + finally: + executor.shutdown(wait=True) diff --git a/tests/test_serve_dx_polish.py b/tests/test_serve_dx_polish.py new file mode 100644 index 000000000..8677827f4 --- /dev/null +++ b/tests/test_serve_dx_polish.py @@ -0,0 +1,205 @@ +"""DX polish for adcp.server.serve — port-collision remediation + +trailing-slash path normalization. + +Both surfaced as cross-cutting friction in 2 of 4 Emma backend tests: + +* Default port=3001 collides silently. Raw ``OSError: [Errno 48] + Address already in use`` deep in ``_bind_reusable_socket`` is what + buyers see — no remediation hint, no breadcrumb to ``port=`` / + ``ADCP_PORT``. +* MCP streamable-http mounts at ``/mcp`` (no trailing slash). Buyer + libraries POSTing to ``/mcp/`` get a 307 redirect; libs that don't + follow redirects on POST silently break (lose the body when the + redirect rewrites POST → GET). + +These tests exercise both fixes at the ASGI layer (path normalize) +and at the bind layer (port-collision projection). +""" + +from __future__ import annotations + +import errno +import socket +from typing import Any + +import pytest + +from adcp.server.serve import _bind_reusable_socket, _wrap_with_path_normalize + +# ---- _bind_reusable_socket EADDRINUSE remediation ---- + + +def test_bind_reusable_socket_friendly_eaddrinuse() -> None: + """When the requested port is bound by another process, the raw + ``OSError`` gets projected to a remediation-bearing message that + points at ``port=`` / ``ADCP_PORT``. Without this fix, every Emma + backend tester wasted ~10 minutes hunting a 30-second + misconfiguration.""" + # Bind a holder socket on an ephemeral port to guarantee + # EADDRINUSE on the second bind. + holder = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + holder.bind(("127.0.0.1", 0)) + holder.listen(1) + busy_port = holder.getsockname()[1] + try: + with pytest.raises(OSError) as exc_info: + _bind_reusable_socket("127.0.0.1", busy_port) + finally: + holder.close() + + msg = str(exc_info.value) + # The remediation must cite both the port number AND a concrete + # next step the adopter can take (port= or ADCP_PORT). Otherwise + # we've just rewritten the error without making it actionable. + assert str(busy_port) in msg + assert "port=" in msg or "ADCP_PORT" in msg + assert exc_info.value.errno == errno.EADDRINUSE + + +def test_bind_reusable_socket_other_oserror_passes_through() -> None: + """Errors that aren't EADDRINUSE (permission denied, address not + available, etc.) MUST pass through unchanged so adopters debugging + a different problem don't get a misleading port-collision message + pointing at the wrong fix.""" + # 0.0.0.0 is bindable; -1 isn't (negative port). The error class + # depends on the platform — we assert pass-through behavior, NOT + # a specific errno. + with pytest.raises((OverflowError, OSError)) as exc_info: + _bind_reusable_socket("127.0.0.1", -1) + msg = str(exc_info.value) + # Specifically MUST NOT carry the EADDRINUSE remediation phrase + # — that would be a false flag pointing the adopter at the wrong + # knob. + assert "ADCP_PORT" not in msg + assert "Pick a different port" not in msg + + +# ---- _wrap_with_path_normalize trailing-slash strip ---- + + +class _CapturingApp: + """Minimal ASGI app that records the path it was invoked with.""" + + def __init__(self) -> None: + self.paths: list[str] = [] + self.raw_paths: list[Any] = [] + + async def __call__(self, scope: dict[str, Any], receive: Any, send: Any) -> None: + self.paths.append(scope.get("path", "")) + self.raw_paths.append(scope.get("raw_path")) + await send({"type": "http.response.start", "status": 204, "headers": []}) + await send({"type": "http.response.body", "body": b"", "more_body": False}) + + +@pytest.mark.asyncio +async def test_path_normalize_strips_trailing_slash_on_mcp() -> None: + """``/mcp/`` → ``/mcp`` BEFORE the inner app sees it. No 307, + body preserved (the app handles the request inline).""" + app = _CapturingApp() + wrapped = _wrap_with_path_normalize(app) + + scope = { + "type": "http", + "method": "POST", + "path": "/mcp/", + "raw_path": b"/mcp/", + "headers": [], + } + + async def _recv() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + sent: list[dict[str, Any]] = [] + + async def _send(msg: dict[str, Any]) -> None: + sent.append(msg) + + await wrapped(scope, _recv, _send) + assert app.paths == ["/mcp"] + assert app.raw_paths == [b"/mcp"] + # Inner app still runs (the wrapped middleware doesn't 307). + assert any(m["type"] == "http.response.start" for m in sent) + + +@pytest.mark.asyncio +async def test_path_normalize_leaves_root_slash_alone() -> None: + """The root path ``/`` MUST NOT be rewritten to ``''`` — that + would route a health check to the empty string and cause 404s. + The middleware only touches non-root trailing slashes.""" + app = _CapturingApp() + wrapped = _wrap_with_path_normalize(app) + + scope = { + "type": "http", + "method": "GET", + "path": "/", + "raw_path": b"/", + "headers": [], + } + + async def _recv() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + sent: list[dict[str, Any]] = [] + + async def _send(msg: dict[str, Any]) -> None: + sent.append(msg) + + await wrapped(scope, _recv, _send) + assert app.paths == ["/"] + + +@pytest.mark.asyncio +async def test_path_normalize_passes_through_path_without_trailing_slash() -> None: + """``/mcp`` (no trailing slash) is the canonical form — the + middleware must not touch it, just pass through unchanged.""" + app = _CapturingApp() + wrapped = _wrap_with_path_normalize(app) + + scope = { + "type": "http", + "method": "POST", + "path": "/mcp", + "raw_path": b"/mcp", + "headers": [], + } + + async def _recv() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + async def _send(msg: dict[str, Any]) -> None: + pass + + await wrapped(scope, _recv, _send) + assert app.paths == ["/mcp"] + + +@pytest.mark.asyncio +async def test_path_normalize_does_not_mutate_outer_scope() -> None: + """The middleware copies the scope before mutating it — without + this, ASGI's contract that adjacent middlewares get an unmodified + scope is violated. Regression guard against a future refactor that + edits scope in-place.""" + app = _CapturingApp() + wrapped = _wrap_with_path_normalize(app) + + original_scope: dict[str, Any] = { + "type": "http", + "method": "POST", + "path": "/mcp/", + "raw_path": b"/mcp/", + "headers": [], + } + snapshot = dict(original_scope) + + async def _recv() -> dict[str, Any]: + return {"type": "http.request", "body": b"", "more_body": False} + + async def _send(msg: dict[str, Any]) -> None: + pass + + await wrapped(original_scope, _recv, _send) + assert original_scope == snapshot, ( + "_wrap_with_path_normalize mutated its outer scope arg; " + "subsequent middlewares would see the rewritten path" + )