diff --git a/examples/hello_seller.py b/examples/hello_seller.py index fc379eb8..237aa4d3 100644 --- a/examples/hello_seller.py +++ b/examples/hello_seller.py @@ -18,6 +18,44 @@ framework's protocol tools * Call ``get_products``: returns one product * Call ``create_media_buy``: returns the success envelope + +# Minimum valid buyer payloads + +The seller-side stubs below ARE valid; the bit that's not obvious is +what the BUYER must send. Each AdCP request type has fields the +schema requires that aren't on the example response. Common ones: + +* ``idempotency_key`` (mutating tools) — string, ``min_length=16``, + buyer-chosen replay-window key. Pad shorter test values with a + random suffix (``"emma-test-create-media-buy-001"``). +* ``buying_mode`` on ``GetProductsRequest`` — ``"brief"`` | + ``"signal"`` | ``"audience"`` | ``"performance"``. +* ``account`` on most mutating tools — ``{"account_id": "..."}``. +* ``packages`` on ``CreateMediaBuyRequest`` — non-empty list with + ``buyer_ref`` + ``products`` + ``budget``. +* ``destinations`` on ``ActivateSignalRequest`` — non-empty list of + ``{"type": "platform", "platform": "..."}``. + +Minimum valid ``create_media_buy`` payload:: + + { + "account": {"account_id": "hello"}, + "buyer_ref": "buyer-1", + "promoted_offering": "shoes", + "packages": [{ + "buyer_ref": "pkg-1", + "products": ["display-rotation"], + "budget": {"total": 100, "currency": "USD"} + }], + "po_number": "PO-1", + "idempotency_key": "buyer-create-mb-001-padding", + "start_time": "2026-06-01T00:00:00Z", + "end_time": "2026-06-30T23:59:59Z" + } + +For the full set of required fields per tool, see +``schemas/cache/media-buy/create-media-buy-request.json`` (and the +corresponding files for each tool) in the SDK distribution. """ from __future__ import annotations diff --git a/examples/hello_seller_creative.py b/examples/hello_seller_creative.py index a15b7d20..e3beaa93 100644 --- a/examples/hello_seller_creative.py +++ b/examples/hello_seller_creative.py @@ -67,8 +67,9 @@ def build_creative( 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``. + message is on ``req.message``; the requested format is on + ``req.target_format_id`` (or ``req.target_format_ids`` for + multi-format builds). """ # Real adopters call their generation API here; this stub # synthesizes a placeholder URL for the example. diff --git a/examples/hello_seller_signals.py b/examples/hello_seller_signals.py index 6127081b..d1756762 100644 --- a/examples/hello_seller_signals.py +++ b/examples/hello_seller_signals.py @@ -98,13 +98,28 @@ def activate_signal( ``{task_id, status: "submitted"}`` envelope synchronously, and runs ``_async_activation`` in the background. """ + # ``ActivateSignalRequest`` carries the signal reference on + # ``signal_agent_segment_id`` (the canonical spec field — the + # segment ID a buyer activates against). The catalog returned + # by ``get_signals`` may use ``signal_id`` for the buyer-facing + # name, but the activation request keys on + # ``signal_agent_segment_id``. + segment_id = getattr(req, "signal_agent_segment_id", "unknown") + # Buyer-supplied destinations list — required (min_length=1) + # and unbounded; we echo back one deployment per destination. + destinations = getattr(req, "destinations", []) or [] return { "deployments": [ { - "destination_platform": getattr(req, "destination_platform", "the-trade-desk"), - "deployment_id": f"dep-{getattr(req, 'signal_id', 'unknown')}", + "destination_platform": ( + getattr(d, "platform", None) + or (d.get("platform") if isinstance(d, dict) else None) + or "the-trade-desk" + ), + "deployment_id": f"dep-{segment_id}", "status": "active", } + for d in (destinations or [{"platform": "the-trade-desk"}]) ] } diff --git a/scripts/run_emma_matrix.sh b/scripts/run_emma_matrix.sh new file mode 100755 index 00000000..dc0255bd --- /dev/null +++ b/scripts/run_emma_matrix.sh @@ -0,0 +1,278 @@ +#!/usr/bin/env bash +# Run the four Emma backend tests in parallel via headless `claude -p`. +# Each backend boots in its own /tmp/emma-* working dir; reports land in +# /tmp/emma-matrix-/ and the aggregated punch list prints to +# stdout when all four complete. +# +# Usage: +# ./scripts/run_emma_matrix.sh # run all 4 in parallel +# ./scripts/run_emma_matrix.sh sales # one backend by name +# ./scripts/run_emma_matrix.sh sales signals # specific subset +# +# Available backends: sales, signals, audiostack, stability +# +# Requires: +# * `claude` CLI on PATH +# * `uv` on PATH +# * SDK at /Users/brianokelley/conductor/adcp-client-python (path can be +# overridden via $ADCP_SDK_PATH) + +set -uo pipefail + +ADCP_SDK_PATH="${ADCP_SDK_PATH:-/Users/brianokelley/conductor/adcp-client-python}" +TIMESTAMP="$(date +%Y%m%d-%H%M%S)" +REPORT_DIR="/tmp/emma-matrix-${TIMESTAMP}" +mkdir -p "${REPORT_DIR}" + +if [[ ! -d "${ADCP_SDK_PATH}/src/adcp/decisioning" ]]; then + echo "ERROR: SDK not found at ${ADCP_SDK_PATH}" >&2 + echo " Override with ADCP_SDK_PATH=/path/to/adcp-client-python" >&2 + exit 1 +fi + +# ---- per-backend briefs ---- + +brief_sales() { + cat < +**Time to first create_media_buy**: +**Per-specialism filter narrowed correctly**: Y/N +### What worked smoothly +### Friction (P0/P1/P2) +### Tools tested (each ✅/❌) +- get_products +- create_media_buy +- update_media_buy +- sync_creatives +- get_media_buy_delivery +- tools/list narrowing (sales only, no leaks) +### What I'd ship as PRs +### Verbatim error messages worth fixing +EOF +} + +brief_signals() { + cat <&2 + # ``--add-dir`` grants the agent read+write under the per-backend + # working dir AND read of the SDK source (so it can find + # ``hello_seller_*.py`` references and follow imports for + # ``DecisioningPlatform`` / ``CreativeManifest`` / etc.). + # + # ``--dangerously-skip-permissions`` bypasses the per-tool approval + # prompts. Headless ``claude -p`` has no human approver — without + # this flag every ``uv``/``Bash`` invocation hangs as "requires + # approval" and the brief never gets started. Safe here because: + # 1) the script is run-from-disk by an authenticated developer, and + # 2) the agent is sandboxed to the working dir + SDK read via + # ``--add-dir`` regardless of this flag. + # + # Path canonicalization note (macOS): ``/tmp`` symlinks to + # ``/private/tmp``. Pass the realpath so the agent's allow-list + # checks succeed against the resolved path (Claude Code's + # canonicalizer compares pre-realpath agent input against post- + # realpath allow-list entries; pre-resolving here sidesteps the + # mismatch). + local resolved_workdir + resolved_workdir="$(cd "${workdir}" && pwd -P)" + ${brief_fn} | claude -p \ + --add-dir "${resolved_workdir}" \ + --add-dir "${ADCP_SDK_PATH}" \ + --dangerously-skip-permissions \ + > "${report}" 2>&1 + echo "[${name}] done" >&2 +} + +# Default to all 4; allow subsetting via positional args. +if [[ $# -eq 0 ]]; then + BACKENDS=(sales signals audiostack stability) +else + BACKENDS=("$@") +fi + +PIDS=() +for backend in "${BACKENDS[@]}"; do + case "${backend}" in + sales) run_backend sales brief_sales & PIDS+=($!) ;; + signals) run_backend signals brief_signals & PIDS+=($!) ;; + audiostack) run_backend audiostack brief_audiostack & PIDS+=($!) ;; + stability) run_backend stability brief_stability & PIDS+=($!) ;; + *) + echo "ERROR: unknown backend ${backend}" >&2 + echo " available: sales, signals, audiostack, stability" >&2 + exit 1 + ;; + esac +done + +echo "Running ${#PIDS[@]} backends in parallel; PIDs: ${PIDS[*]}" >&2 +echo "Reports → ${REPORT_DIR}/" >&2 + +# Wait for all and capture exit codes. +EXIT_CODE=0 +for pid in "${PIDS[@]}"; do + if ! wait "${pid}"; then + EXIT_CODE=1 + echo "WARN: backend pid ${pid} exited non-zero" >&2 + fi +done + +# Aggregate. +AGG="${REPORT_DIR}/aggregate.md" +{ + echo "# Emma Backend Matrix — ${TIMESTAMP}" + echo + echo "Run from ${ADCP_SDK_PATH}" + echo + for backend in "${BACKENDS[@]}"; do + if [[ -f "${REPORT_DIR}/${backend}.md" ]]; then + cat "${REPORT_DIR}/${backend}.md" + echo + echo "---" + echo + fi + done +} > "${AGG}" + +echo +echo "=================================================================" +echo "Aggregated report: ${AGG}" +echo "=================================================================" +cat "${AGG}" +exit "${EXIT_CODE}" diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index 791df858..f3fcce28 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -355,29 +355,65 @@ def _internal_error_message(method_name: str, exc: BaseException) -> str: def _internal_error_details(exc: BaseException) -> dict[str, Any]: - """Build the wire-side ``details.caused_by`` for an INTERNAL_ERROR + """Build the wire-side ``details`` payload 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 + ``details.caused_by`` carries 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. + + **ValidationError special case** (Stability AI Emma P1 from the + post-#340 matrix): when the platform method raises a pydantic + ``ValidationError`` directly — typically because the seller's + code constructed an invalid response model — the wire used to + say "see details for cause" with no actual details. We now also + emit ``details.validation_errors`` carrying the narrowed + field-path list (using + :func:`adcp.types.error_narrowing.narrow_union_errors` to filter + discriminated-union noise). The buyer agent gets actionable + field paths; the seller dev sees the same in their wire log. + Pydantic ValidationError is the only common adopter exception + where a structured field list is meaningful, so we don't + generalize this to other exception types. """ raw = str(exc) if len(raw) > _INTERNAL_ERROR_DETAIL_CHARS: raw = raw[: _INTERNAL_ERROR_DETAIL_CHARS - 3] + "..." - return { + details: dict[str, Any] = { "caused_by": { "type": type(exc).__name__, "message": raw, } } + # Try to import lazily so a future refactor that splits the + # validation tooling can't ripple through the dispatch layer. + try: + from pydantic import ValidationError + + from adcp.types.error_narrowing import narrow_union_errors + except Exception: + return details + if isinstance(exc, ValidationError): + try: + errors_list = exc.errors( + include_input=False, + include_context=False, + include_url=False, + ) + details["validation_errors"] = list(narrow_union_errors(errors_list)) + except Exception: + # Defensive — never let a narrowing bug 500 the wire. + # The caused_by.message already carries the truncated + # repr; adopters can still triage via server logs. + pass + return details # --------------------------------------------------------------------------- diff --git a/src/adcp/server/base.py b/src/adcp/server/base.py index d099d6b6..26905863 100644 --- a/src/adcp/server/base.py +++ b/src/adcp/server/base.py @@ -8,72 +8,86 @@ from abc import ABC from dataclasses import dataclass, field -from typing import TYPE_CHECKING, Any, Generic, TypeVar +from typing import Any, Generic, TypeVar from pydantic import BaseModel -from adcp.types import Error - -if TYPE_CHECKING: - from adcp.types import ( - AcquireRightsRequest, - ActivateSignalRequest, - BuildCreativeRequest, - CalibrateContentRequest, - CheckGovernanceRequest, - ComplyTestControllerRequest, - ContextMatchRequest, - CreateCollectionListRequest, - CreateContentStandardsRequest, - CreateMediaBuyRequest, - CreatePropertyListRequest, - DeleteCollectionListRequest, - DeletePropertyListRequest, - GetAccountFinancialsRequest, - GetAdcpCapabilitiesRequest, - GetBrandIdentityRequest, - GetCollectionListRequest, - GetContentStandardsRequest, - GetCreativeDeliveryRequest, - GetCreativeFeaturesRequest, - GetMediaBuyArtifactsRequest, - GetMediaBuyDeliveryRequest, - GetMediaBuysRequest, - GetPlanAuditLogsRequest, - GetProductsRequest, - GetPropertyListRequest, - GetRightsRequest, - GetSignalsRequest, - IdentityMatchRequest, - ListAccountsRequest, - ListCollectionListsRequest, - ListContentStandardsRequest, - ListCreativeFormatsRequest, - ListCreativesRequest, - ListPropertyListsRequest, - LogEventRequest, - PreviewCreativeRequest, - ProvidePerformanceFeedbackRequest, - ReportPlanOutcomeRequest, - ReportUsageRequest, - SiGetOfferingRequest, - SiInitiateSessionRequest, - SiSendMessageRequest, - SiTerminateSessionRequest, - SyncAccountsRequest, - SyncAudiencesRequest, - SyncCatalogsRequest, - SyncCreativesRequest, - SyncEventSourcesRequest, - SyncGovernanceRequest, - SyncPlansRequest, - UpdateCollectionListRequest, - UpdateContentStandardsRequest, - UpdateMediaBuyRequest, - UpdatePropertyListRequest, - UpdateRightsRequest, - ValidateContentDeliveryRequest, - ) +# Request types are imported at module scope (NOT under TYPE_CHECKING) +# so that ``typing.get_type_hints(method)`` resolves every ADCPHandler +# baseline method's ``params`` annotation at runtime. The dispatcher's +# ``adcp.server.mcp_tools._resolve_params_pydantic_model`` walks these +# hints to deserialise wire-shape dicts into typed Pydantic models; +# without runtime visibility, ``get_type_hints`` raises ``NameError``, +# the resolver swallows the exception (warning + dict-fallback), and +# the framework's own ``get_adcp_capabilities`` warns on every cold +# boot about an annotation NO ADOPTER CAN FIX (Emma cross-cutting P1 +# from the post-#340 matrix run — sales/signals/stability all flagged +# this). +# +# Same root cause as PR #338's handler.py fix; we missed base.py +# because the framework methods inherit ``not_supported`` defaults +# and the wire-dispatch failure mode wasn't visible until the +# resolver-warning bump in #338 surfaced it as console noise. +from adcp.types import ( + AcquireRightsRequest, + ActivateSignalRequest, + BuildCreativeRequest, + CalibrateContentRequest, + CheckGovernanceRequest, + ComplyTestControllerRequest, + ContextMatchRequest, + CreateCollectionListRequest, + CreateContentStandardsRequest, + CreateMediaBuyRequest, + CreatePropertyListRequest, + DeleteCollectionListRequest, + DeletePropertyListRequest, + Error, + GetAccountFinancialsRequest, + GetAdcpCapabilitiesRequest, + GetBrandIdentityRequest, + GetCollectionListRequest, + GetContentStandardsRequest, + GetCreativeDeliveryRequest, + GetCreativeFeaturesRequest, + GetMediaBuyArtifactsRequest, + GetMediaBuyDeliveryRequest, + GetMediaBuysRequest, + GetPlanAuditLogsRequest, + GetProductsRequest, + GetPropertyListRequest, + GetRightsRequest, + GetSignalsRequest, + IdentityMatchRequest, + ListAccountsRequest, + ListCollectionListsRequest, + ListContentStandardsRequest, + ListCreativeFormatsRequest, + ListCreativesRequest, + ListPropertyListsRequest, + LogEventRequest, + PreviewCreativeRequest, + ProvidePerformanceFeedbackRequest, + ReportPlanOutcomeRequest, + ReportUsageRequest, + SiGetOfferingRequest, + SiInitiateSessionRequest, + SiSendMessageRequest, + SiTerminateSessionRequest, + SyncAccountsRequest, + SyncAudiencesRequest, + SyncCatalogsRequest, + SyncCreativesRequest, + SyncEventSourcesRequest, + SyncGovernanceRequest, + SyncPlansRequest, + UpdateCollectionListRequest, + UpdateContentStandardsRequest, + UpdateMediaBuyRequest, + UpdatePropertyListRequest, + UpdateRightsRequest, + ValidateContentDeliveryRequest, +) @dataclass diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index d95f97c8..8ea7e6b9 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -1036,6 +1036,16 @@ def _register_tool( from adcp.exceptions import ADCPError from adcp.server.translate import translate_error + # Lazy import — decisioning is optional for non-platform handlers, + # but when present its ``AdcpError`` carries structured ``details`` + # (caused_by, validation_errors) that ``translate_error`` now + # understands. AudioStack Emma P0: pre-fix this exception class + # propagated to FastMCP's default handler and ``details`` was lost. + try: + from adcp.decisioning.types import AdcpError as DecisioningAdcpError # noqa: N813 + except Exception: + DecisioningAdcpError = None # type: ignore[assignment,misc] # noqa: N806 + async def fn(**kwargs: Any) -> dict[str, Any]: # Caller identity: FastMCP does not expose an authenticated principal # at the SDK level (``Context.client_id`` is a session hint, not an @@ -1082,6 +1092,19 @@ async def _call_handler() -> Any: # the code via either structuredContent.adcp_error (if populated) # or the text-fallback path. raise translate_error(exc, protocol="mcp") from exc + except Exception as exc: + # Decisioning ``AdcpError`` is NOT a subclass of + # ``adcp.exceptions.ADCPError`` (different class hierarchy + # — ``adcp.decisioning.types.AdcpError``). Without this + # branch it propagated to FastMCP's default exception + # handler and ``details`` was lost on the wire. AudioStack + # Emma P0 confirmed pre-fix. + if DecisioningAdcpError is not None and isinstance(exc, DecisioningAdcpError): + # ``# type: ignore[arg-type]`` because mypy can't see + # that ``translate_error`` accepts decisioning AdcpError + # via the lazy-import branch. + raise translate_error(exc, protocol="mcp") from exc # type: ignore[arg-type] + raise if hasattr(result, "model_dump"): return result.model_dump(mode="json", exclude_none=True) # type: ignore[no-any-return] if isinstance(result, dict): diff --git a/src/adcp/server/translate.py b/src/adcp/server/translate.py index 847a1882..cb13f278 100644 --- a/src/adcp/server/translate.py +++ b/src/adcp/server/translate.py @@ -25,6 +25,7 @@ from __future__ import annotations +import json from typing import Any, Literal from urllib.parse import urlparse @@ -143,6 +144,15 @@ def translate_error( if proto not in ("mcp", "a2a"): raise ValueError(f"protocol must be 'mcp' or 'a2a', got {protocol!r}") + # Lazy import — ``adcp.decisioning.types`` pulls in the decisioning + # graph, which translate.py shouldn't load at module-import time. + # Match against ``BaseException`` here and let the elif body do the + # actual isinstance check via the imported name. + try: + from adcp.decisioning.types import AdcpError as DecisioningAdcpError # noqa: N813 + except Exception: + DecisioningAdcpError = None # type: ignore[assignment,misc] # noqa: N806 + # Extract structured fields from the input field: str | None = None if isinstance(exc, Error): @@ -153,6 +163,21 @@ def translate_error( recovery = _recovery_for_code(code) errors = None field = exc.field + elif DecisioningAdcpError is not None and isinstance(exc, DecisioningAdcpError): + # Decisioning-layer ``AdcpError`` (distinct from + # ``adcp.exceptions.ADCPError``) carries its own structured + # ``details`` (e.g. ``caused_by`` from the INTERNAL_ERROR wrap, + # ``validation_errors`` from #341's narrowing). AudioStack Emma + # P0: pre-fix this branch was missing entirely, so AdcpError + # propagated to FastMCP's default exception path and ``details`` + # never reached the wire. + code = exc.code + message = exc.args[0] if exc.args else "" + suggestion = exc.suggestion + recovery = exc.recovery + details = exc.details or None + errors = None + field = exc.field elif isinstance(exc, ADCPError): code = _error_code_for_exception(exc) message = exc.message @@ -167,11 +192,17 @@ def translate_error( if errors: first = errors[0] field = getattr(first, "field", None) + # ADCPTaskError errors[0].details (e.g. validation_errors + # from create_tool_caller's INVALID_REQUEST projection) + # should also reach MCP — A2A already passes them via the + # ``errors`` array, but MCP only sees the first error's + # field/message. Embed details too. + details = getattr(first, "details", None) else: raise TypeError(f"Expected ADCPError or Error, got {type(exc).__name__}") if proto == "mcp": - return _to_mcp(code, message, suggestion=suggestion, field=field) + return _to_mcp(code, message, suggestion=suggestion, field=field, details=details) return _to_a2a( code, message, @@ -188,6 +219,7 @@ def _to_mcp( *, suggestion: str | None = None, field: str | None = None, + details: dict[str, Any] | None = None, ) -> ToolError: """Format error as a ToolError for MCP servers. @@ -199,6 +231,14 @@ def _to_mcp( (``^([A-Z_]+)(?:\\[([^\\]]+)\\])?:``) to recover both the AdCP code and the field path — same shape the spec suggests for the JS client. + + 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 + ``details.caused_by`` (and #341's ``validation_errors``) never + reached MCP buyers — only A2A. Now both transports surface the + structured breadcrumb. """ if field: text = f"{code}[{field}]: {message}" @@ -206,6 +246,20 @@ def _to_mcp( text = f"{code}: {message}" 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. + try: + details_json = json.dumps(details, separators=(",", ":"), default=str) + except Exception: + details_json = str(details) + if len(details_json) > 8192: + details_json = details_json[:8189] + "..." + text += f"\nDetails: {details_json}" return ToolError(text) diff --git a/tests/test_decisioning_dispatch.py b/tests/test_decisioning_dispatch.py index f545261f..5725700c 100644 --- a/tests/test_decisioning_dispatch.py +++ b/tests/test_decisioning_dispatch.py @@ -686,6 +686,57 @@ async def get_products(self, req, ctx): assert truncated.endswith("...") +@pytest.mark.asyncio +async def test_invoke_validation_error_surfaces_narrowed_field_paths( + executor: ThreadPoolExecutor, +) -> None: + """When the platform method raises ``pydantic.ValidationError`` + directly — typically because the seller constructed an invalid + response model — the wire ``details`` MUST carry the narrowed + field-path list so the buyer agent sees what failed (Stability AI + Emma P1: pre-fix wire said "see details for cause" with empty + details). Field paths are pulled from + ``ValidationError.errors()`` and run through + ``narrow_union_errors`` to filter discriminated-union noise.""" + from pydantic import BaseModel + + class _ResponseModel(BaseModel): + creative_id: str + width: int + height: int + + class _CrashingPlatform(DecisioningPlatform): + capabilities = DecisioningCapabilities() + accounts = SingletonAccounts(account_id="x") + + async def get_products(self, req, ctx): + # Seller-side bug: building a response with missing fields. + # Realistic shape: an adopter calling + # CreativeManifest(...) with a missing required ``url`` on + # ImageContent. + _ResponseModel.model_validate({"creative_id": "cr-1"}) + + ctx = _build_request_context(ToolContext(), Account(id="x"), None) + with pytest.raises(AdcpError) as exc_info: + await _invoke_platform_method( + _CrashingPlatform(), + "get_products", + _ProductsRequest(), + ctx, + executor=executor, + registry=InMemoryTaskRegistry(), + ) + assert exc_info.value.code == "INTERNAL_ERROR" + # ``caused_by`` still surfaces the exception class for triage. + assert exc_info.value.details["caused_by"]["type"] == "ValidationError" + # NEW: ``validation_errors`` is populated with structured field + # paths so the buyer agent (and the seller's wire log) see the + # actual missing fields. + validation_errors = exc_info.value.details["validation_errors"] + missing_fields = {err["loc"][-1] for err in validation_errors if err["type"] == "missing"} + assert "width" in missing_fields and "height" in missing_fields + + @pytest.mark.asyncio async def test_invoke_arg_projector_signature_drift_projects_invalid_request( executor: ThreadPoolExecutor, diff --git a/tests/test_server_framework.py b/tests/test_server_framework.py index c9398b82..5ccae624 100644 --- a/tests/test_server_framework.py +++ b/tests/test_server_framework.py @@ -628,11 +628,19 @@ def test_get_tool_names(self): @pytest.mark.asyncio async def test_call_tool_invokes_handler(self): - """Test calling a tool invokes the handler method.""" + """Test calling a tool invokes the handler method. + + Sends a minimum-valid ``GetProductsRequest`` payload (just + ``buying_mode``) so the framework's typed-dispatch path actually + invokes the handler. Pre-fix, the framework's TYPE_CHECKING bug + meant the resolver fell back to dict dispatch and any payload + flowed through; post-fix the handler IS reached but typed + validation rejects empty dicts (which is the correct + spec-compliant behavior for ``GetProductsRequest``).""" handler = ADCPHandler() tools = create_mcp_tools(handler) - result = await tools.call_tool("get_products", {}) + result = await tools.call_tool("get_products", {"buying_mode": "brief"}) # Should return the not_supported response as a dict assert result["supported"] is False diff --git a/tests/test_translate.py b/tests/test_translate.py index 2b4836d0..1bac8aec 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -370,3 +370,112 @@ def test_all_transforms_combined(self): assert "brand_manifest" not in result assert "promoted_offerings" not in result assert "campaign_ref" not in result + + +# ---- AdcpError details surfacing on MCP wire (AudioStack Emma P0) ---- + + +class TestAdcpErrorDetailsOnMCP: + """Pre-#341: ``adcp.decisioning.types.AdcpError`` was NOT a subclass + of ``adcp.exceptions.ADCPError``, so ``except ADCPError`` in + ``serve.py`` didn't catch it. The exception propagated to FastMCP's + default handler and ``details`` (carrying ``caused_by`` from the + INTERNAL_ERROR wrap, ``validation_errors`` from #341's narrowing) + was silently dropped. + + Post-fix: ``translate_error`` recognizes decisioning ``AdcpError``, + extracts ``details``, and ``_to_mcp`` serialises them as a + ``\\nDetails: `` line in the ToolError text payload. Buyer + libs can split on the prefix and ``json.loads`` the tail.""" + + def test_adcp_error_details_serialized_on_mcp_wire(self) -> None: + """When dispatch raises ``AdcpError(INTERNAL_ERROR, + details={"caused_by": ...})``, the MCP ToolError payload + carries ``\\nDetails: {"caused_by": ...}`` so buyer agents can + parse the structured breadcrumb.""" + import json as _json + + from adcp.decisioning.types import AdcpError as DecisioningAdcpError + from adcp.server.translate import translate_error + + exc = DecisioningAdcpError( + "INTERNAL_ERROR", + message="Platform method 'build_creative' raised AttributeError", + recovery="terminal", + details={ + "caused_by": { + "type": "AttributeError", + "message": "'dict' object has no attribute 'message'", + } + }, + ) + tool_err = translate_error(exc, protocol="mcp") + text = str(tool_err.args[0]) if tool_err.args else str(tool_err) + assert "INTERNAL_ERROR:" in text + assert "build_creative" in text + # The structured breadcrumb is parseable from the text payload. + assert "\nDetails: " in text + details_json = text.split("\nDetails: ", 1)[1] + parsed = _json.loads(details_json) + assert parsed["caused_by"]["type"] == "AttributeError" + + def test_validation_errors_reach_mcp_buyer_via_details(self) -> None: + """#341's ``ValidationError`` narrowing populates + ``details.validation_errors`` on the dispatch's wrap. That + list now flows through to MCP buyers — pre-fix it only reached + A2A buyers because the MCP path dropped ``details``.""" + import json as _json + + from adcp.decisioning.types import AdcpError as DecisioningAdcpError + from adcp.server.translate import translate_error + + exc = DecisioningAdcpError( + "INTERNAL_ERROR", + message="Platform method 'build_creative' raised ValidationError", + recovery="terminal", + details={ + "caused_by": {"type": "ValidationError", "message": "..."}, + "validation_errors": [ + { + "type": "missing", + "loc": ["assets", "hero", "ImageAsset", "width"], + "msg": "Field required", + }, + { + "type": "missing", + "loc": ["assets", "hero", "ImageAsset", "height"], + "msg": "Field required", + }, + ], + }, + ) + tool_err = translate_error(exc, protocol="mcp") + text = str(tool_err.args[0]) if tool_err.args else str(tool_err) + details_json = text.split("\nDetails: ", 1)[1] + parsed = _json.loads(details_json) + # Both narrowed errors reach the buyer with full field paths. + assert len(parsed["validation_errors"]) == 2 + 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: + """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.""" + 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", + message="huge", + recovery="terminal", + details=big, + ) + 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] + assert len(details_part) <= 8192 + assert details_part.endswith("...")