diff --git a/examples/v3_reference_seller/src/app.py b/examples/v3_reference_seller/src/app.py index faa0bb76b..df9652d5c 100644 --- a/examples/v3_reference_seller/src/app.py +++ b/examples/v3_reference_seller/src/app.py @@ -147,16 +147,15 @@ def main() -> None: asgi_middleware=[ (SubdomainTenantMiddleware, {"router": router}), ], - # Schema-driven validation in strict mode on both sides: - # the dispatcher validates every request against the bundled - # AdCP JSON schemas before the platform method runs, and - # validates every response after it returns. Bugs like the - # ``pricing_options`` shape regression that shipped in the - # initial v3 ref seller are caught at boot / first call - # rather than during a buyer's storyboard run. Adopters - # forking this entrypoint inherit the strict default — drop - # to ``responses="warn"`` only when you have a deliberate - # reason to ship spec-divergent responses. + # Schema-driven validation in strict mode on both sides. + # This is the framework default since DX#8 (strict by default + # to catch ``pricing_options``-class bugs that ``extra="allow"`` + # Pydantic models silently swallow), but pinned explicitly here + # so the reference seller's posture is self-evident from the + # serve call. Adopters forking this entrypoint can drop to + # ``responses="warn"`` if they have a deliberate reason to + # ship spec-divergent responses; they cannot escape detection + # by simply omitting the kwarg. validation=ValidationHookConfig(requests="strict", responses="strict"), # Wire the anti-façade traffic counters. Storyboard runners # poll ``GET /_debug/traffic`` to assert the platform actually diff --git a/src/adcp/server/a2a_server.py b/src/adcp/server/a2a_server.py index 1723ad52e..cafe46c04 100644 --- a/src/adcp/server/a2a_server.py +++ b/src/adcp/server/a2a_server.py @@ -47,10 +47,14 @@ from a2a.server.tasks.task_store import TaskStore from adcp.server.serve import ContextFactory, SkillMiddleware - from adcp.validation.client_hooks import ValidationHookConfig from collections.abc import Callable # noqa: E402 +from adcp.validation.client_hooks import ( # noqa: E402 + SERVER_DEFAULT_VALIDATION, + ValidationHookConfig, +) + MessageParser = Callable[[RequestContext], tuple[str | None, dict[str, Any]]] """Callable that extracts ``(skill_name, params)`` from an incoming A2A :class:`RequestContext`. @@ -125,7 +129,7 @@ def __init__( middleware: Sequence[SkillMiddleware] | None = None, message_parser: MessageParser | None = None, advertise_all: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = SERVER_DEFAULT_VALIDATION, ) -> None: self._handler = handler self._context_factory = context_factory @@ -600,7 +604,7 @@ def create_a2a_server( middleware: Sequence[SkillMiddleware] | None = None, message_parser: MessageParser | None = None, advertise_all: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = SERVER_DEFAULT_VALIDATION, ) -> Any: """Create an A2A Starlette application from an ADCP handler. @@ -678,6 +682,14 @@ def create_a2a_server( ``skills`` list and in the executor's tool-caller registry. Turn on for spec-compliance storyboards or when the agent deliberately wants clients to see a ``not_supported`` tool. + validation: :class:`ValidationHookConfig` enabling schema + validation of every request and response against the + bundled AdCP JSON schemas. Defaults to + :data:`~adcp.validation.client_hooks.SERVER_DEFAULT_VALIDATION` + (strict on both sides). Pass + ``ValidationHookConfig(responses="warn")`` to log+continue + on response drift, or ``validation=None`` to disable + validation entirely. Returns: A Starlette app ready to be run with uvicorn. diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index acbf50cf0..d9c4a8b27 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -33,6 +33,19 @@ async def get_adcp_capabilities(self, params, context=None): create_tool_caller, get_tools_for_handler, ) +from adcp.validation.client_hooks import ( + SERVER_DEFAULT_VALIDATION as DEFAULT_VALIDATION, +) +from adcp.validation.client_hooks import ( + ValidationHookConfig, +) + +# Re-exported as ``adcp.server.serve.DEFAULT_VALIDATION`` for adopters who +# want a non-magic name when constructing their own +# ``ValidationHookConfig`` overrides. The canonical definition lives in +# :mod:`adcp.validation.client_hooks` so both the server-side and any +# future server-creation seam can share one constant without a circular +# import via this module. if TYPE_CHECKING: from collections.abc import Sequence @@ -44,7 +57,6 @@ async def get_adcp_capabilities(self, params, context=None): from adcp.server.a2a_server import MessageParser from adcp.server.test_controller import TestControllerStore - from adcp.validation.client_hooks import ValidationHookConfig @dataclass(frozen=True) @@ -413,7 +425,7 @@ def serve( advertise_all: bool = False, max_request_size: int | None = None, streaming_responses: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, enable_debug_endpoints: bool = False, debug_traffic_source: Callable[[], dict[str, int]] | None = None, base_url: str | None = None, @@ -543,16 +555,24 @@ def serve( their specialism SHOULD pass it. description: Optional human-readable description surfaced in the discovery manifest's per-agent ``description`` field. - validation: Optional :class:`ValidationHookConfig` enabling - schema validation of every request and response against the + validation: :class:`ValidationHookConfig` enabling schema + validation of every request and response against the bundled AdCP JSON schemas. ``requests="strict"`` raises ``VALIDATION_ERROR`` before the handler runs on a malformed payload; ``responses="strict"`` raises after the handler - returns when the response shape drifts from spec. Sellers - who want their server to enforce wire conformance pass - ``ValidationHookConfig(requests="strict", responses="strict")``; - the default ``None`` keeps validation off (zero overhead). - Applies to both MCP and A2A transports. + returns when the response shape drifts from spec. + + **Defaults to** :data:`DEFAULT_VALIDATION` (strict on both + sides) — wire-conformance by default. This catches the + class of bug that shipped the ``pricing_options`` + regression past Pydantic ``extra="allow"`` silently + swallowing an unknown shape. Adopters mid-migration who + need response drift to warn rather than fail pass + ``ValidationHookConfig(responses="warn")``; adopters who + want validation off entirely pass + ``ValidationHookConfig(requests="off", responses="off")`` + or ``validation=None``. Applies to both MCP and A2A + transports. Security: This function does NOT configure authentication. In production, @@ -936,7 +956,7 @@ def _serve_mcp( advertise_all: bool = False, max_request_size: int | None = None, streaming_responses: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, base_url: str | None = None, specialisms: list[str] | None = None, description: str | None = None, @@ -1064,7 +1084,7 @@ def _serve_a2a( message_parser: MessageParser | None = None, advertise_all: bool = False, max_request_size: int | None = None, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, base_url: str | None = None, specialisms: list[str] | None = None, description: str | None = None, @@ -1135,7 +1155,7 @@ def _build_mcp_and_a2a_app( advertise_all: bool = False, max_request_size: int | None = None, streaming_responses: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, base_url: str | None = None, specialisms: list[str] | None = None, description: str | None = None, @@ -1272,7 +1292,7 @@ def _serve_mcp_and_a2a( advertise_all: bool = False, max_request_size: int | None = None, streaming_responses: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, base_url: str | None = None, specialisms: list[str] | None = None, description: str | None = None, @@ -1350,7 +1370,7 @@ def create_mcp_server( middleware: Sequence[SkillMiddleware] | None = None, advertise_all: bool = False, streaming_responses: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, ) -> Any: """Create a FastMCP server from an ADCP handler without starting it. @@ -1486,7 +1506,7 @@ def _register_handler_tools( context_factory: ContextFactory | None = None, middleware: Sequence[SkillMiddleware] | None = None, advertise_all: bool = False, - validation: ValidationHookConfig | None = None, + validation: ValidationHookConfig | None = DEFAULT_VALIDATION, ) -> None: """Register all ADCP tools from a handler onto a FastMCP server.""" # Freeze middleware ordering at registration time. Tuple both guards diff --git a/src/adcp/validation/__init__.py b/src/adcp/validation/__init__.py index d1ec913b3..49f42835c 100644 --- a/src/adcp/validation/__init__.py +++ b/src/adcp/validation/__init__.py @@ -16,6 +16,7 @@ from __future__ import annotations from adcp.validation.client_hooks import ( + SERVER_DEFAULT_VALIDATION, DebugLogEntry, ValidationHookConfig, ValidationMode, @@ -76,6 +77,7 @@ "build_validation_error", # Client hooks "DebugLogEntry", + "SERVER_DEFAULT_VALIDATION", "ValidationHookConfig", "ValidationMode", "resolve_validation_modes", diff --git a/src/adcp/validation/client_hooks.py b/src/adcp/validation/client_hooks.py index 8c1d0af9a..bbc40c908 100644 --- a/src/adcp/validation/client_hooks.py +++ b/src/adcp/validation/client_hooks.py @@ -58,6 +58,21 @@ class ValidationHookConfig: responses: ValidationMode | None = None +#: Server-side default — strict on both request and response sides. +#: Used by :func:`adcp.server.serve` and the underlying ``create_*_server`` +#: factories when the adopter does not pass ``validation=`` explicitly. +#: Strict-by-default makes the SDK enforce wire conformance: a malformed +#: request fails before the handler runs (``VALIDATION_ERROR``); a +#: spec-divergent response fails after the handler returns. Catches the +#: class of bug that ``extra="allow"`` Pydantic models silently swallow +#: (e.g. the ``pricing_options`` regression). Adopters opt out via +#: ``ValidationHookConfig(responses="warn")`` (warn-only) or +#: ``validation=None`` (off entirely). +SERVER_DEFAULT_VALIDATION: ValidationHookConfig = ValidationHookConfig( + requests="strict", responses="strict" +) + + class DebugLogEntry(TypedDict, total=False): """Append-only entry shape for the ``debug_logs`` list threaded by the client and server call paths. ``total=False`` so callers can diff --git a/tests/integration/test_a2a_context_id.py b/tests/integration/test_a2a_context_id.py index 578291b11..42f7d03b5 100644 --- a/tests/integration/test_a2a_context_id.py +++ b/tests/integration/test_a2a_context_id.py @@ -127,6 +127,10 @@ async def _running_server(handler: ADCPHandler, observer: _Observer) -> AsyncIte name="integration-test-agent", port=port, message_parser=observer.parser, + # Stub handler that returns synthetic responses; the test + # exercises A2A context-id wire plumbing, not spec-shape + # request payloads. Opt out of the strict server default. + validation=None, ) config = uvicorn.Config(app, host="127.0.0.1", port=port, log_level="warning") server = uvicorn.Server(config) diff --git a/tests/integration/test_a2a_wire_compat.py b/tests/integration/test_a2a_wire_compat.py index 71c3e4843..1f755b519 100644 --- a/tests/integration/test_a2a_wire_compat.py +++ b/tests/integration/test_a2a_wire_compat.py @@ -58,7 +58,13 @@ def _pick_free_port() -> int: @asynccontextmanager async def _running_server() -> AsyncIterator[str]: port = _pick_free_port() - app = create_a2a_server(_EchoHandler(), name="wire-compat-agent", port=port) + app = create_a2a_server( + _EchoHandler(), + name="wire-compat-agent", + port=port, + # Wire-compat plumbing test — stub echo handler. + validation=None, + ) config = uvicorn.Config(app, host="127.0.0.1", port=port, log_level="warning") server = uvicorn.Server(config) task = asyncio.create_task(server.serve()) diff --git a/tests/test_a2a_server.py b/tests/test_a2a_server.py index 4fec61e4e..5e5a32307 100644 --- a/tests/test_a2a_server.py +++ b/tests/test_a2a_server.py @@ -17,12 +17,32 @@ from adcp.server import ADCPHandler from adcp.server.a2a_server import ( - ADCPAgentExecutor, + ADCPAgentExecutor as _ADCPAgentExecutor, +) +from adcp.server.a2a_server import ( _build_agent_card, create_a2a_server, ) from adcp.server.test_controller import TestControllerError, TestControllerStore + +def ADCPAgentExecutor(*args: Any, **kwargs: Any) -> _ADCPAgentExecutor: # noqa: N802 + """Test wrapper that defaults ``validation=None``. + + The framework defaults to strict-by-default wire-conformance + validation; this test module focuses on transport plumbing + (dispatch, middleware composition, parser hooks, context echo) + and uses minimal stub handlers that do not return fully + spec-conformant responses. Opting out of validation here keeps the + transport contract under test without forcing every stub to grow + full ``Product`` / ``adcp.idempotency`` payloads. Tests that + specifically want to assert validation behavior pass an explicit + ``validation=`` kwarg, which overrides this default. + """ + kwargs.setdefault("validation", None) + return _ADCPAgentExecutor(*args, **kwargs) + + # Backwards-compat fixture aliases: tests construct these at the # 0.3-era Pydantic call sites (``DataPart(data=...)``, ``TextPart(text=...)``, # ``Part(root=data_part)``). In 1.0 everything is a proto ``Part`` with a @@ -1134,7 +1154,7 @@ async def noop_mw(skill_name, params, context, call_next): app = create_a2a_server(_TestHandler(), name="mw-test", middleware=[noop_mw]) handler = _extract_default_request_handler(app) executor = handler.agent_executor - assert isinstance(executor, ADCPAgentExecutor) + assert isinstance(executor, _ADCPAgentExecutor) assert executor._middleware == (noop_mw,) @@ -1319,7 +1339,7 @@ def my_parser(ctx: RequestContext) -> tuple[str | None, dict[str, Any]]: app = create_a2a_server(_TestHandler(), name="parser-test", message_parser=my_parser) handler = _extract_default_request_handler(app) executor = handler.agent_executor - assert isinstance(executor, ADCPAgentExecutor) + assert isinstance(executor, _ADCPAgentExecutor) assert executor._message_parser is my_parser diff --git a/tests/test_handler_typevar.py b/tests/test_handler_typevar.py index 471340087..e51dca4f3 100644 --- a/tests/test_handler_typevar.py +++ b/tests/test_handler_typevar.py @@ -298,7 +298,10 @@ class _TypedAgent(ADCPHandler[_TypedContext]): async def get_adcp_capabilities(self, params, context=None): return {"adcp": {"major_versions": [3]}, "supported_protocols": ["media_buy"]} - executor = ADCPAgentExecutor(_TypedAgent()) + # validation=None opts out of strict-by-default wire-conformance — + # this test asserts dispatch plumbing under a TypeVar'd handler, + # not the schema shape of the stub's response. + executor = ADCPAgentExecutor(_TypedAgent(), validation=None) msg = Message( message_id="m-1", role=Role.user, @@ -384,7 +387,8 @@ def _factory(meta): account_id="acct-42", ) - executor = ADCPAgentExecutor(_AccountAwareAgent(), context_factory=_factory) + # See note on _TypedAgent above re: validation=None opt-out. + executor = ADCPAgentExecutor(_AccountAwareAgent(), context_factory=_factory, validation=None) msg = Message( message_id="m-1", role=Role.user, diff --git a/tests/test_mcp_middleware_composition.py b/tests/test_mcp_middleware_composition.py index fc6ce3295..31fa3315c 100644 --- a/tests/test_mcp_middleware_composition.py +++ b/tests/test_mcp_middleware_composition.py @@ -149,6 +149,12 @@ async def handler_and_client() -> Any: handler, name="test-agent", context_factory=_build_context, + # Tests assert middleware composition / context-factory plumbing + # against a stub handler that returns minimal payloads — opt out + # of the framework's strict-by-default wire-conformance check + # so a non-spec-conformant stub response doesn't get rewritten + # into a VALIDATION_ERROR before the assertion runs. + validation=None, ) # Force stateless JSON responses. Production deployments mount the # MCP app behind a reverse proxy; this test covers that shape. @@ -435,6 +441,7 @@ async def inner(skill_name, params, context, call_next): name="mw-test", context_factory=_build_context, middleware=[outer, inner], + validation=None, # transport plumbing test, not wire-conformance ) mcp.settings.stateless_http = True mcp.settings.json_response = True diff --git a/tests/test_serve_validation_default.py b/tests/test_serve_validation_default.py new file mode 100644 index 000000000..5244a609f --- /dev/null +++ b/tests/test_serve_validation_default.py @@ -0,0 +1,86 @@ +"""Pin the framework default for ``serve(validation=...)`` — +``ValidationHookConfig(requests="strict", responses="strict")``. + +The framework promotes wire-conformance to the default to catch the +class of bug that shipped the ``pricing_options`` regression: a +Pydantic model with ``extra="allow"`` silently swallowed the unknown +shape past type-validation, and only an end-to-end schema check at the +wire boundary surfaced it. Strict-by-default puts that check on every +adopter's serve path. + +These tests pin: + +* ``inspect`` shows the default kwarg as ``DEFAULT_VALIDATION`` (signature + contract — adopters/IDEs see strict in autocomplete). +* The default value is the canonical + :data:`adcp.validation.client_hooks.SERVER_DEFAULT_VALIDATION` — + same constant in serve, a2a_server, and the validation module. +* Adopters opt out via ``validation=None`` (off entirely) or + ``ValidationHookConfig(responses="warn")`` (response warn-only). +""" + +from __future__ import annotations + +import inspect + +from adcp.server.a2a_server import create_a2a_server +from adcp.server.serve import ( + DEFAULT_VALIDATION, + create_mcp_server, + serve, +) +from adcp.validation import ValidationHookConfig +from adcp.validation.client_hooks import SERVER_DEFAULT_VALIDATION + + +def test_default_validation_is_strict_strict() -> None: + """The canonical default is strict requests and strict responses. + + The constant is the load-bearing source of truth; if this changes, + every server-side call site that defaults to it changes posture in + lockstep. + """ + assert isinstance(DEFAULT_VALIDATION, ValidationHookConfig) + assert DEFAULT_VALIDATION.requests == "strict" + assert DEFAULT_VALIDATION.responses == "strict" + + +def test_default_validation_is_canonical() -> None: + """``adcp.server.serve.DEFAULT_VALIDATION`` aliases the validation + module's :data:`SERVER_DEFAULT_VALIDATION` — there is one source of + truth, not two parallel constants drifting independently. + """ + assert DEFAULT_VALIDATION is SERVER_DEFAULT_VALIDATION + + +def test_serve_signature_default_is_strict() -> None: + """``serve(validation=...)`` advertises the strict default in its + signature so IDEs and ``help(serve)`` show the right value. + """ + sig = inspect.signature(serve) + param = sig.parameters["validation"] + assert param.default is DEFAULT_VALIDATION + assert isinstance(param.default, ValidationHookConfig) + assert param.default.requests == "strict" + assert param.default.responses == "strict" + + +def test_create_mcp_server_signature_default_is_strict() -> None: + """Same contract on ``create_mcp_server`` — the lower-level seam + adopters compose with their own auth / ASGI middleware also + defaults strict. + """ + sig = inspect.signature(create_mcp_server) + param = sig.parameters["validation"] + assert param.default is DEFAULT_VALIDATION + + +def test_create_a2a_server_signature_default_is_strict() -> None: + """Same contract on the A2A side — both transports default strict + so an adopter calling ``serve(handler, transport="a2a")`` and an + adopter calling ``create_a2a_server(handler)`` get identical + posture without wiring it twice. + """ + sig = inspect.signature(create_a2a_server) + param = sig.parameters["validation"] + assert param.default is SERVER_DEFAULT_VALIDATION diff --git a/tests/test_serve_validation_passthrough.py b/tests/test_serve_validation_passthrough.py index 497ffe014..f5b3eb3db 100644 --- a/tests/test_serve_validation_passthrough.py +++ b/tests/test_serve_validation_passthrough.py @@ -62,10 +62,13 @@ def _spy(handler_arg: Any, method: str, **kwargs: Any) -> Any: ), f"validation kwarg dropped on the way to create_tool_caller: {captured!r}" -def test_serve_validation_default_is_none() -> None: - """When ``serve(validation=)`` is omitted, tool callers see - ``validation=None`` — i.e., off (zero overhead). Confirms the - plumbing doesn't silently force a default mode. +def test_serve_validation_default_is_strict() -> None: + """When ``serve(validation=)`` is omitted, tool callers receive the + framework default :data:`~adcp.server.serve.DEFAULT_VALIDATION` — + strict on both request and response sides. The framework no longer + ships an "off-by-default" path; silent drift past + ``extra="allow"`` Pydantic models cost more than the validation + overhead. """ handler = _StubHandler({"products": []}) captured: list[ValidationHookConfig | None] = [] @@ -78,10 +81,35 @@ def _spy(handler_arg: Any, method: str, **kwargs: Any) -> Any: with patch.object(_serve_mod, "create_tool_caller", side_effect=_spy): create_mcp_server(handler) + assert captured, "create_tool_caller was never called" + for cfg in captured: + assert isinstance( + cfg, ValidationHookConfig + ), f"validation default is not a ValidationHookConfig: {cfg!r}" + assert cfg.requests == "strict", f"expected requests=strict, got {cfg!r}" + assert cfg.responses == "strict", f"expected responses=strict, got {cfg!r}" + + +def test_serve_validation_explicit_none_disables() -> None: + """Passing ``validation=None`` explicitly threads ``None`` through to + the tool caller — the documented opt-out for adopters who want + validation entirely off. + """ + handler = _StubHandler({"products": []}) + captured: list[ValidationHookConfig | None] = [] + real_factory = _serve_mod.create_tool_caller + + def _spy(handler_arg: Any, method: str, **kwargs: Any) -> Any: + captured.append(kwargs.get("validation")) + return real_factory(handler_arg, method, **kwargs) + + with patch.object(_serve_mod, "create_tool_caller", side_effect=_spy): + create_mcp_server(handler, validation=None) + assert captured, "create_tool_caller was never called" assert all( c is None for c in captured - ), f"validation default leaked a non-None config: {captured!r}" + ), f"validation=None should propagate as None, got: {captured!r}" @pytest.mark.asyncio diff --git a/tests/test_server_caller_identity.py b/tests/test_server_caller_identity.py index a393f37a0..1a6dfb975 100644 --- a/tests/test_server_caller_identity.py +++ b/tests/test_server_caller_identity.py @@ -133,7 +133,7 @@ async def create_media_buy( return {"media_buy_id": f"mb_{self.calls}", "status": "completed"} seller = Seller() - executor = ADCPAgentExecutor(seller) + executor = ADCPAgentExecutor(seller, validation=None) key = str(uuid.uuid4()) # Simulate two successive A2A calls from the same authenticated buyer. @@ -160,7 +160,7 @@ async def create_media_buy( return {"media_buy_id": f"mb_{self.calls}"} seller = Seller() - executor = ADCPAgentExecutor(seller) + executor = ADCPAgentExecutor(seller, validation=None) key = str(uuid.uuid4()) params = {"idempotency_key": key, "brand": "acme"} @@ -189,7 +189,7 @@ async def create_media_buy( return {"media_buy_id": f"mb_{self.calls}"} seller = Seller() - executor = ADCPAgentExecutor(seller) + executor = ADCPAgentExecutor(seller, validation=None) key = str(uuid.uuid4()) params = {"idempotency_key": key, "brand": "acme"} @@ -218,7 +218,11 @@ async def get_products( seen["identity"] = context.caller_identity if context else None return {"products": []} - executor = ADCPAgentExecutor(CaptureHandler()) + # Transport-plumbing test — opt out of strict-by-default + # wire-conformance so the stub's empty-products response and + # missing required request fields don't short-circuit the + # dispatch under test. + executor = ADCPAgentExecutor(CaptureHandler(), validation=None) # Build a minimal A2A request with an authenticated user. class _Req: