diff --git a/examples/hello_seller.py b/examples/hello_seller.py index 237aa4d3d..1395c5e24 100644 --- a/examples/hello_seller.py +++ b/examples/hello_seller.py @@ -4,8 +4,12 @@ * :class:`DecisioningCapabilities` declared on the class body * :class:`SingletonAccounts` for the dev/single-tenant case -* Three platform methods (``get_products``, ``create_media_buy``, - ``get_media_buy_delivery``) — all sync, sync return path +* Five required ``sales-non-guaranteed`` methods (``get_products``, + ``create_media_buy``, ``update_media_buy``, ``sync_creatives``, + ``get_media_buy_delivery``) — all sync, sync return path. The full + required set is enforced at server boot by ``validate_platform`` + via :data:`REQUIRED_METHODS_PER_SPECIALISM` — omitting any of the + five fails fast with INVALID_REQUEST. Run:: @@ -75,12 +79,11 @@ class HelloSeller(DecisioningPlatform): """The canonical minimal v6.0 sales-non-guaranteed adopter. - Implements the three sync methods every sales-* specialism - requires for a buyer to discover, transact, and read delivery. - Production sellers would add ``update_media_buy`` and - ``sync_creatives`` to satisfy the full sales-non-guaranteed - contract; this example focuses on the common-path subset that - fits in one screen. + Implements all five required methods of ``sales-non-guaranteed`` + (the full contract per :data:`REQUIRED_METHODS_PER_SPECIALISM`): + ``get_products``, ``create_media_buy``, ``update_media_buy``, + ``sync_creatives``, ``get_media_buy_delivery``. ``validate_platform`` + runs at boot and fails fast on any missing method. """ capabilities = DecisioningCapabilities( @@ -217,9 +220,13 @@ def get_media_buy_delivery( req: Any, ctx: RequestContext[Any], ) -> dict[str, Any]: - """Stub delivery snapshot — flat zeros.""" + """Stub delivery snapshot — flat zeros. + + Wire field is ``media_buy_deliveries`` per + ``schemas/cache/media-buy/get-media-buy-delivery-response.json``. + """ return { - "deliveries": [ + "media_buy_deliveries": [ { "media_buy_id": getattr(req, "media_buy_id", "mb_unknown"), "totals": {"impressions": 0, "spend": 0.0}, diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index f3fcce281..fda1b94aa 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -364,6 +364,20 @@ def _internal_error_details(exc: BaseException) -> dict[str, Any]: (typo-shaped) from ``KeyError`` (missing-config-shaped) from ``ConnectionError`` (network-shaped) at a glance. + **``caused_by.type`` is a debug breadcrumb, not a wire contract.** + The value is Python's exception class name verbatim + (``"AttributeError"``, ``"KeyError"``, ``"ValidationError"``). + Buyers built against the JS SDK won't see Python-flavoured class + names from JS sellers — only Python sellers leak Python types. + Treat this field as "hint to the seller dev reading their own + server logs," NOT as something to branch on programmatically + cross-language. The AdCP spec at ``schemas/cache/core/error.json`` + keeps ``details`` as ``additionalProperties: true`` so this is + spec-compliant; it's just not portable. Buyer agents that want + structured retry/fix/abandon classification should read + ``recovery`` (terminal/correctable/transient) which IS the + cross-language contract. + 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 diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index 8ea7e6b95..538bd461c 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -769,6 +769,19 @@ def _run_mcp_http(mcp: Any, *, transport: str, max_request_size: int | None = No sock = _bind_reusable_socket(host, port) try: + # One INFO line at the bind boundary so adopters know exactly + # what URL the buyer should hit. uvicorn's default startup logs + # are filtered/quieted in many configurations; this line is + # framework-controlled and always lands. Emma signals/sales + # backend tests both flagged silent-boot as P1 friction. + mcp_path = "/mcp" if transport == "streamable-http" else "/sse" + logger.info( + "MCP listening on http://%s:%s%s (transport=%s)", + host, + port, + mcp_path, + transport, + ) config = uvicorn.Config(app, log_level=log_level) server = uvicorn.Server(config) @@ -816,6 +829,10 @@ def _serve_a2a( app = _wrap_with_size_limit(app, max_request_size) sock = _bind_reusable_socket("0.0.0.0", resolved_port) try: + # Same bind-boundary INFO as the MCP path so A2A adopters + # also see one framework-controlled line confirming the + # listener is up. + logger.info("A2A listening on http://0.0.0.0:%s/", resolved_port) config = uvicorn.Config(app) server = uvicorn.Server(config) import anyio diff --git a/src/adcp/server/translate.py b/src/adcp/server/translate.py index cb13f2787..578ce4c4b 100644 --- a/src/adcp/server/translate.py +++ b/src/adcp/server/translate.py @@ -234,11 +234,23 @@ def _to_mcp( When ``details`` is non-empty, the JSON-serialised payload is appended after a ``\\nDetails: `` line. Buyer agents can split on - that prefix and ``json.loads`` the rest. AudioStack Emma P0: - pre-fix the wire said "see details for cause" but the dispatch's + that prefix and ``json.loads`` the rest — the result is ALWAYS + valid JSON (truncation/serialization failures emit a sentinel + object, never a bare ``...``). AudioStack Emma P0: pre-fix the + wire said "see details for cause" but the dispatch's ``details.caused_by`` (and #341's ``validation_errors``) never reached MCP buyers — only A2A. Now both transports surface the structured breadcrumb. + + **Bridge, not endpoint.** The protocol-correct shape is MCP's + ``CallToolResult.structuredContent`` (carries ``isError=True`` + AND a structured ``adcp_error`` object on the same envelope — + see ``mcp.types.CallToolResult``). FastMCP's + ``_make_error_result`` (``mcp/server/lowlevel/server.py:467``) + drops ``structuredContent`` for error results, so we can't reach + that channel via FastMCP's ``ToolError`` raise path. Migrating + to lowlevel ``Server.call_tool`` registration would unlock it; + until then this text-suffix is the working bridge. """ if field: text = f"{code}[{field}]: {message}" @@ -247,20 +259,86 @@ def _to_mcp( if suggestion: text += f"\nSuggestion: {suggestion}" if details: - # Truncation: the entire MCP error is one ``text`` field, so a - # huge details dict bloats every error response. Cap the JSON - # payload at 8 KB — generous enough for the structured - # breadcrumb shapes (caused_by + validation_errors typically - # under 2 KB) but bounded against an adopter who fills details - # with raw repr or DB query strings. + text += f"\nDetails: {_serialize_details_for_mcp(details)}" + return ToolError(text) + + +#: Cap on the JSON-serialised ``details`` payload appended to MCP +#: ToolError text. Generous enough for typical +#: ``caused_by`` + ``validation_errors`` shapes (under 2 KB) and +#: bounded against an adopter who fills ``details`` with raw repr +#: or DB query strings. Not configurable today; if an adopter +#: needs more, an env-var escape hatch is the right next step. +_MCP_DETAILS_MAX_BYTES = 8192 + + +def _serialize_details_for_mcp(details: dict[str, Any]) -> str: + """Serialise ``details`` to a JSON string suitable for embedding + in the MCP ToolError text payload. + + The output is ALWAYS valid JSON — even when truncation fires + or ``json.dumps`` raises. Buyer agents can split on the + ``\\nDetails: `` prefix and ``json.loads`` the tail without + branching on serialization quality. Truncation is signalled via + the ``_truncated`` field on the sentinel object so buyers can + surface a "details elided; see server logs" UX hint. + + Pre-fix (PR #341 ship): truncation emitted a bare ``...`` suffix + on the JSON tail, which made the buyer's ``json.loads`` throw + ``JSONDecodeError`` with no signal that the cause was + server-side truncation. ad-tech-protocol-expert called this + out as a follow-up before the wire shape became de-facto + convention. + """ + try: + details_json = json.dumps(details, separators=(",", ":"), default=str) + except Exception: + # Non-serializable details (rare — ``default=str`` catches + # most). Emit a sentinel so the buyer's parse still + # succeeds. ``str(details)`` may also raise on circular refs + # — guarded with try/except + a fallback empty partial. try: - details_json = json.dumps(details, separators=(",", ":"), default=str) + partial = str(details)[: _MCP_DETAILS_MAX_BYTES // 2] except Exception: - details_json = str(details) - if len(details_json) > 8192: - details_json = details_json[:8189] + "..." - text += f"\nDetails: {details_json}" - return ToolError(text) + partial = "" + return json.dumps( + { + "_truncated": True, + "_reason": "non_serializable", + "_partial": partial, + }, + separators=(",", ":"), + ) + if len(details_json) <= _MCP_DETAILS_MAX_BYTES: + return details_json + # Truncation: emit a sentinel object (always valid JSON). + # ``_partial`` carries as much of the original payload as fits + # inside the cap; buyers that want the full payload pull it + # from server logs (still in ``logger.exception`` traces). + # + # Iterate to fit: JSON encoding of ``_partial`` adds backslash + # escaping (each `"` becomes `\\"`, each `\\` becomes `\\\\`), + # so a naive headroom calc undershoots when the partial contains + # quotes or backslashes. Halve until the encoded sentinel fits. + cut = _MCP_DETAILS_MAX_BYTES - 64 # rough headroom for sentinel keys + while cut > 0: + encoded = json.dumps( + { + "_truncated": True, + "_reason": "size", + "_partial": details_json[:cut], + }, + separators=(",", ":"), + ) + if len(encoded) <= _MCP_DETAILS_MAX_BYTES: + return encoded + cut = max(0, cut - max(64, (len(encoded) - _MCP_DETAILS_MAX_BYTES) * 2)) + # Cut hit zero — emit a no-partial sentinel so the buyer still + # sees that something was truncated. + return json.dumps( + {"_truncated": True, "_reason": "size", "_partial": ""}, + separators=(",", ":"), + ) def _to_a2a( diff --git a/tests/test_hello_seller_integration.py b/tests/test_hello_seller_integration.py index 79bfc086d..1ba3d3a6f 100644 --- a/tests/test_hello_seller_integration.py +++ b/tests/test_hello_seller_integration.py @@ -163,8 +163,12 @@ async def test_get_media_buy_delivery_returns_zeros( ) resp = await handler.get_media_buy_delivery(req, ToolContext()) assert isinstance(resp, dict) - assert len(resp["deliveries"]) == 1 - assert resp["deliveries"][0]["totals"]["impressions"] == 0 + # Wire field is ``media_buy_deliveries`` per + # ``schemas/cache/media-buy/get-media-buy-delivery-response.json``. + # Pre-fix the example used the wrong key (``deliveries``); pin the + # spec field name here so future drift fails the test. + assert len(resp["media_buy_deliveries"]) == 1 + assert resp["media_buy_deliveries"][0]["totals"]["impressions"] == 0 @pytest.mark.asyncio diff --git a/tests/test_translate.py b/tests/test_translate.py index 1bac8aec1..efef8b63b 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -458,15 +458,21 @@ def test_validation_errors_reach_mcp_buyer_via_details(self) -> None: loc_fields = {err["loc"][-1] for err in parsed["validation_errors"]} assert loc_fields == {"width", "height"} - def test_huge_details_truncated_at_8kb(self) -> None: + def test_huge_details_truncated_with_parseable_sentinel(self) -> None: """A pathological ``details`` payload (huge repr, DB query - string) gets truncated to 8KB so the MCP error response stays - bounded. Defense-in-depth against an adopter who fills - ``details`` with raw debug payloads.""" + string) gets truncated to 8 KB so the MCP error response + stays bounded. The truncated payload is ALWAYS valid JSON — + emits a ``{"_truncated": true, "_reason": "size", "_partial": + "..."}`` sentinel so buyer agents can ``json.loads`` and + branch on the marker (ad-tech-protocol-expert P1 follow-up + from PR #341 — pre-fix the bare ``...`` suffix made buyer + ``json.loads`` throw ``JSONDecodeError`` with no signal of + why).""" + import json as _json + from adcp.decisioning.types import AdcpError as DecisioningAdcpError from adcp.server.translate import translate_error - # Build a details dict that JSON-encodes to >8 KB. big = {"blob": "X" * 10_000} exc = DecisioningAdcpError( "INTERNAL_ERROR", @@ -478,4 +484,37 @@ def test_huge_details_truncated_at_8kb(self) -> None: text = str(tool_err.args[0]) if tool_err.args else str(tool_err) details_part = text.split("\nDetails: ", 1)[1] assert len(details_part) <= 8192 - assert details_part.endswith("...") + # Critical: the truncated payload MUST be valid JSON. + parsed = _json.loads(details_part) + assert parsed["_truncated"] is True + assert parsed["_reason"] == "size" + assert "_partial" in parsed + + def test_unserializable_details_emits_sentinel(self) -> None: + """When ``json.dumps`` itself raises (a circular reference or + other unrepresentable shape that even ``default=str`` can't + coerce), the function emits a parseable sentinel rather than + letting the failure propagate or returning malformed JSON.""" + import json as _json + + from adcp.decisioning.types import AdcpError as DecisioningAdcpError + from adcp.server.translate import translate_error + + # Circular reference defeats both ``json.dumps`` and the + # ``default=str`` fallback (str() can't represent the cycle + # either — it raises ``ValueError: Circular reference``). + circular: dict[str, object] = {} + circular["self"] = circular + exc = DecisioningAdcpError( + "INTERNAL_ERROR", + message="circular", + recovery="terminal", + details=circular, + ) + tool_err = translate_error(exc, protocol="mcp") + text = str(tool_err.args[0]) if tool_err.args else str(tool_err) + details_part = text.split("\nDetails: ", 1)[1] + # Even on serialization failure, the output is parseable. + parsed = _json.loads(details_part) + assert parsed["_truncated"] is True + assert parsed["_reason"] == "non_serializable"