From 83b56861e1ba2babb611f49b91dad08481894e06 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 11 May 2026 08:24:24 -0400 Subject: [PATCH] feat(server): pre-adapter validation against legacy schema (stage 4b2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the buyer claims a legacy version that routes through the adapter path, validate the input against the buyer's claimed schema *before* the adapter runs. Legacy field paths surface in errors — easier to act on than v3 paths after translation. Flow when wire_version in LEGACY_ADAPTER_VERSIONS: 1. Lookup adapter; INVALID_REQUEST if missing. 2. (new) Validate params against the legacy schema: - strict: raise VALIDATION_ERROR with claimed_version in details - warn: log the violation and let the adapter try - off / no config: skip (zero-overhead path preserved) 3. Run adapter.adapt_request(params). 4. Post-adapter validation against the SDK pin (existing). Stage 4b1 bundled v2.5 schemas; this wires them into the dispatch path. Full versioned-schema-validation port complete. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/server/mcp_tools.py | 55 +++-- .../test_dispatcher_pre_adapter_validation.py | 197 ++++++++++++++++++ 2 files changed, 240 insertions(+), 12 deletions(-) create mode 100644 tests/test_dispatcher_pre_adapter_validation.py diff --git a/src/adcp/server/mcp_tools.py b/src/adcp/server/mcp_tools.py index 221c6229..da81fa00 100644 --- a/src/adcp/server/mcp_tools.py +++ b/src/adcp/server/mcp_tools.py @@ -2028,10 +2028,13 @@ async def call_tool(params: dict[str, Any], context: ToolContext | None = None) wire_version = None # Legacy-version routing: if the buyer claims a version handled - # via the adapter path (e.g. ``"2.5"``), translate the request - # to current-version shape before validation. The output is then - # validated against the SDK pin's schema, so a buggy translator - # surfaces as ``INVALID_REQUEST`` with a field-level pointer. + # via the adapter path (e.g. ``"2.5"``), validate the params + # against the legacy schema first, *then* translate to the + # current shape. Pre-adapter validation surfaces structural + # errors with the legacy schema's field paths — far easier + # for the buyer to act on than a v3 field-path error after a + # confusing translation. Post-adapter validation (further + # down) catches translator bugs against the SDK pin. legacy_adapter: Any = None if wire_version in LEGACY_ADAPTER_VERSIONS: legacy_adapter = get_legacy_adapter(wire_version, method_name) @@ -2051,6 +2054,38 @@ async def call_tool(params: dict[str, Any], context: ToolContext | None = None) ) ], ) + + # Pre-adapter validation against the legacy schema. + # Only runs when validation is enabled at all + # (``request_mode != off`` AND a config is supplied) — keeps + # the zero-overhead path for adopters who haven't opted in. + # ``strict`` rejects; ``warn`` logs and proceeds so the + # adapter still gets to translate (matching the existing + # post-adapter contract). + if request_mode is not None and request_mode != "off": + pre_outcome = validate_request(method_name, params, version=wire_version) + if not pre_outcome.valid: + summary = format_issues(pre_outcome.issues) + if request_mode == "strict": + payload = build_adcp_validation_error_payload( + method_name, "request", pre_outcome.issues + ) + # Annotate with the wire version so adopter + # telemetry knows which schema rejected. + payload_details = dict(payload.get("details") or {}) + payload_details["claimed_version"] = wire_version + payload["details"] = payload_details + raise ADCPTaskError( + operation=method_name, + errors=[Error(**payload)], + ) + logger.warning( + "Schema validation warning (pre-adapter %s) for %s: %s", + wire_version, + method_name, + summary, + ) + try: params = legacy_adapter.adapt_request(params) except Exception as exc: @@ -2067,14 +2102,10 @@ async def call_tool(params: dict[str, Any], context: ToolContext | None = None) ) ], ) from exc - # ``adapt_request`` produced a current-version dict; - # validate it against the SDK pin's schema, not the buyer's - # claimed legacy version. This is the variable Stage 4b - # (real legacy schema bundle) extends: pre-adapter input - # gets validated against ``wire_version``, post-adapter - # output against ``None`` (SDK pin) as today. The - # ``post_adapter_validator_version`` name documents which - # of the two roles this value plays. + # Adapter output is validated against the SDK pin + # (catches translator bugs with v3 field paths). The + # ``post_adapter_validator_version`` name documents + # which side of the adapter this value plays. post_adapter_validator_version: str | None = None else: post_adapter_validator_version = wire_version diff --git a/tests/test_dispatcher_pre_adapter_validation.py b/tests/test_dispatcher_pre_adapter_validation.py new file mode 100644 index 00000000..aae0c74f --- /dev/null +++ b/tests/test_dispatcher_pre_adapter_validation.py @@ -0,0 +1,197 @@ +"""Stage 4b2 tests: pre-adapter validation against the legacy schema. + +When the buyer's claimed version routes through the legacy adapter, +``create_tool_caller`` validates the input against that legacy version's +schema *before* the adapter runs. Structural errors surface with the +legacy schema's field paths — far easier for the buyer than a v3 field +path after a confusing translation. + +These tests exercise the strict + warn modes, plus the no-validation +fallthrough. +""" + +from __future__ import annotations + +import logging +from typing import Any + +import pytest + +from adcp.exceptions import ADCPTaskError +from adcp.server.base import ADCPHandler, ToolContext +from adcp.server.mcp_tools import create_tool_caller +from adcp.validation.client_hooks import ValidationHookConfig + + +class _SyncCreativesHandler(ADCPHandler[Any]): + def __init__(self) -> None: + self.received: list[dict[str, Any]] = [] + + async def sync_creatives(self, params: dict[str, Any], ctx: ToolContext) -> dict[str, Any]: + self.received.append(params) + return {"creatives": []} + + +# --------------------------------------------------------------------------- +# Strict mode — v2.5 schema violations raise INVALID_REQUEST +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_strict_rejects_v2_5_payload_with_v2_5_field_path() -> None: + """A v2.5 buyer's payload that fails v2.5 validation is rejected + *before* the adapter runs. The error reports the v2.5 schema's + field path — far easier for the buyer to act on than a v3 field + path after a confusing translation. + + v2.5 ``sync_creatives`` types ``creatives`` as ``array``; sending + an int triggers a type error from the v2.5 validator. + """ + handler = _SyncCreativesHandler() + caller = create_tool_caller( + handler, + "sync_creatives", + validation=ValidationHookConfig(requests="strict"), + ) + + with pytest.raises(ADCPTaskError) as exc_info: + await caller({"adcp_version": "2.5", "creatives": 42}) + + err = exc_info.value.errors[0] + # ``build_adcp_validation_error_payload`` returns + # ``VALIDATION_ERROR`` (matches the post-adapter contract). + assert err.code == "VALIDATION_ERROR" + # The v2.5 schema reported the type error at /creatives. + assert "/creatives" in err.message + # Wire version preserved in details so adopter telemetry can + # attribute the failure to a legacy claim. + assert err.details is not None + assert err.details.get("claimed_version") == "2.5" + # Handler never ran. + assert handler.received == [] + + +@pytest.mark.asyncio +async def test_strict_rejects_payload_valid_in_v2_5_but_missing_v3_required() -> None: + """A v2.5 buyer's payload that's valid in v2.5 but missing a + v3-required field passes pre-adapter validation, gets translated, + and is then rejected by post-adapter v3 validation. The error + surfaces the v3 schema's field path so the buyer knows what to + supply. + """ + handler = _SyncCreativesHandler() + caller = create_tool_caller( + handler, + "sync_creatives", + validation=ValidationHookConfig(requests="strict"), + ) + + # v2.5 only requires ``creatives``; this is structurally fine. + # v3 also requires ``idempotency_key`` + ``account``, so the + # post-adapter v3 check fails. + with pytest.raises(ADCPTaskError) as exc_info: + await caller({"adcp_version": "2.5", "creatives": []}) + + err = exc_info.value.errors[0] + assert err.code == "VALIDATION_ERROR" + # v3 schema reported the missing-field error. + assert "idempotency_key" in err.message or "account" in err.message + assert handler.received == [] + + +# --------------------------------------------------------------------------- +# Warn mode — validation failures log but don't block +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_warn_logs_v2_5_validation_failure_and_proceeds( + caplog: pytest.LogCaptureFixture, +) -> None: + """In ``warn`` mode, a v2.5 schema violation logs but lets the + adapter try anyway. Matches the existing post-adapter warn semantics + so adopters have a consistent escalation path strict ← warn ← off.""" + handler = _SyncCreativesHandler() + caller = create_tool_caller( + handler, + "sync_creatives", + validation=ValidationHookConfig(requests="warn"), + ) + + with caplog.at_level(logging.WARNING): + await caller({"adcp_version": "2.5", "creatives": 42}) + + # Warning logged about pre-adapter validation failure. + assert any("pre-adapter 2.5" in rec.message.lower() for rec in caplog.records), [ + rec.message for rec in caplog.records + ] + + +# --------------------------------------------------------------------------- +# Off — no validation runs (default behaviour preserved) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_no_validation_config_skips_pre_adapter_check() -> None: + """The zero-overhead path: ``validation=None`` (default) bypasses + both pre- and post-adapter validation. Stage 4b2 must not pull + schema-loading into the hot path for adopters who haven't opted in. + """ + handler = _SyncCreativesHandler() + caller = create_tool_caller(handler, "sync_creatives") # no validation= + + # Garbage payload — would fail v2.5 validation if it ran. The + # adapter still gets a chance because validation is off; the v2.5 + # sync_creatives adapter is permissive when ``creatives`` isn't a + # list (returns args unchanged), so the handler sees the original. + await caller({"adcp_version": "2.5", "creatives": 42}) + + assert len(handler.received) == 1 + assert handler.received[0]["creatives"] == 42 + + +# --------------------------------------------------------------------------- +# Adapter-output validation against v3 still runs +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_v2_5_payload_valid_against_v2_5_but_translator_produces_invalid_v3() -> None: + """Belt + braces: even if pre-adapter validation passes, the + *post*-adapter v3 validation catches translator bugs (the contract + Stage 4 already ships). This test pins that the v2.5 pre-check + doesn't replace the v3 post-check.""" + + from adcp.compat.legacy import AdapterPair, _reset_registry_for_tests, register_adapter + + # Register a rogue adapter that returns a v3-invalid dict so the + # post-adapter validator should catch it. + def rogue(payload: dict[str, Any]) -> dict[str, Any]: + return {"creatives": "not-a-list-after-adapt"} + + _reset_registry_for_tests() + try: + register_adapter( + "2.5", + AdapterPair(tool_name="sync_creatives", adapt_request=rogue), + ) + + handler = _SyncCreativesHandler() + # ``strict`` so a v3 validation failure raises; the v2.5 + # pre-check needs to pass (empty creatives is fine in v2.5). + caller = create_tool_caller( + handler, + "sync_creatives", + validation=ValidationHookConfig(requests="strict"), + ) + + with pytest.raises(ADCPTaskError) as exc_info: + await caller({"adcp_version": "2.5", "creatives": []}) + + # The v3 post-adapter validator caught the bad output. + err = exc_info.value.errors[0] + assert err.code in ("INVALID_REQUEST", "VALIDATION_ERROR") + assert handler.received == [] + finally: + _reset_registry_for_tests()