From 36ccf4b884e84ceabdd2e671343a34476ae5906b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 20 Aug 2026 10:03:41 +0200 Subject: [PATCH 1/2] fix(sdk): address beta testing and migration gaps --- src/adcp/migrate/v3_to_v4.py | 55 ++++++++++++- src/adcp/testing/decisioning.py | 72 +++++++++++++++- src/adcp/types/__init__.py | 6 ++ src/adcp/types/_eager.py | 6 ++ src/adcp/types/aliases.py | 12 +++ src/adcp/types/versioned.py | 29 +++++++ tests/fixtures/public_api_snapshot.json | 3 + tests/test_collision_aliases.py | 7 ++ tests/test_migrate_v3_to_v4.py | 104 +++++++++++++++++------- tests/test_testing_decisioning.py | 99 ++++++++++++++++++++++ tests/test_version_scoped_models.py | 56 +++++++++++++ 11 files changed, 418 insertions(+), 31 deletions(-) diff --git a/src/adcp/migrate/v3_to_v4.py b/src/adcp/migrate/v3_to_v4.py index 7762b0c37..3e55fd08e 100644 --- a/src/adcp/migrate/v3_to_v4.py +++ b/src/adcp/migrate/v3_to_v4.py @@ -49,6 +49,7 @@ from __future__ import annotations import argparse +import importlib import json import re import sys @@ -170,8 +171,16 @@ # module. Numbered codegen names cannot be mapped safely by symbol alone: the # same bare name may describe a different schema in another generated module. GENERATED_POC_SOURCE_SYMBOL_MAP: dict[tuple[str, str], str] = { + ("core.account", "GovernanceAgent"): "adcp.types.CoreGovernanceAgent", ("core.account_ref", "AccountReference1"): "adcp.types.AccountIdReference", ("core.account_ref", "AccountReference2"): "adcp.types.InlineAccountReference", + ("core.creative_asset", "CreativeAsset"): "adcp.types.LegacyCreativeAsset", + ("core.creative_filters", "CreativeFilters"): "adcp.types.LegacyCreativeFilters", + ("core.product_filters", "Country"): "adcp.types.ProductFilterCountry", + ("core.product_format_declaration", "ProductFormatDeclaration"): ( + "adcp.types.LegacyProductFormatDeclaration" + ), + ("core.property", "Identifier"): "adcp.types.PropertyIdentifier", ("core.vendor_pricing_option", "VendorPricingOption"): ("adcp.types.VendorPricingOptionUnion"), ("core.vendor_pricing_option", "VendorPricingOption1"): ("adcp.types.CpmVendorPricingOption"), ("core.vendor_pricing_option", "VendorPricingOption2"): ( @@ -196,6 +205,11 @@ ("media_buy.update_media_buy_response", "UpdateMediaBuyResponse2"): ( "adcp.types.LegacyUpdateMediaBuyErrorResponse" ), + ("media_buy.update_media_buy_response", "UpdateMediaBuyResponse1"): ( + "adcp.types.LegacyUpdateMediaBuySuccessResponse" + ), + ("creative.list_creatives_request", "Sort"): "adcp.types.ListCreativesSort", + ("signals.get_signals_response", "Signal"): "adcp.types.GetSignalsSignal", } @@ -210,12 +224,38 @@ ) -def _generated_symbol_replacement(module: str, symbol: str) -> str | None: +def _proposed_generated_symbol_replacement(module: str, symbol: str) -> str | None: return GENERATED_POC_SOURCE_SYMBOL_MAP.get((module, symbol)) or GENERATED_POC_SYMBOL_MAP.get( symbol ) +def _replacement_is_identical(module: str, symbol: str, replacement: str) -> bool: + """Verify that a private class and its proposed public target are identical.""" + if not module or not replacement.startswith("adcp.types."): + return False + try: + source_module = importlib.import_module(f"adcp.types.generated_poc.{module}") + public_module = importlib.import_module("adcp.types") + source = getattr(source_module, symbol) + public = getattr(public_module, replacement.removeprefix("adcp.types.")) + except (AttributeError, ImportError): + return False + return source is public + + +def _generated_symbol_replacement(module: str, symbol: str) -> str | None: + replacement = _proposed_generated_symbol_replacement(module, symbol) + if replacement is None or not _replacement_is_identical(module, symbol, replacement): + return None + return replacement + + +def _unsafe_replacement_hint(module: str, symbol: str, replacement: str) -> str: + source = f"adcp.types.generated_poc.{module}.{symbol}" + return f"SKIP: source {source} is not identical to {replacement} — " "manual rewrite required" + + # Regex for numbered Assets direct imports (``Assets5``, ``Assets14``, etc). # Bare ``Assets`` (no digits) is a legitimate base class alias; the # regex requires at least one digit to avoid false positives. @@ -575,6 +615,19 @@ def scan_file( # the import-path fix covers it. if auto_apply and symbol in NUMBERED_ASSETS_RENAMES: continue + proposed = _proposed_generated_symbol_replacement(module, symbol) + if proposed is not None: + findings.append( + Finding( + kind="flag_private", + path=str(path), + line=lineno, + column=sym_col, + before=symbol, + hint=_unsafe_replacement_hint(module, symbol, proposed), + ) + ) + continue findings.append( Finding( kind="flag_private", diff --git a/src/adcp/testing/decisioning.py b/src/adcp/testing/decisioning.py index c303d0eb5..fb3adf726 100644 --- a/src/adcp/testing/decisioning.py +++ b/src/adcp/testing/decisioning.py @@ -25,6 +25,7 @@ from __future__ import annotations +import warnings from contextlib import asynccontextmanager from typing import TYPE_CHECKING, Any, Literal from urllib.parse import urlparse @@ -49,7 +50,12 @@ ) from adcp.server.auth import BearerTokenAuth from adcp.server.helpers import ResponseEnhancer - from adcp.server.serve import ASGIMiddlewareEntry, ContextFactory, SkillMiddleware + from adcp.server.serve import ( + ASGIMiddlewareEntry, + ContextFactory, + LifespanHook, + SkillMiddleware, + ) from adcp.server.spec_compat import PreValidationHooks @@ -148,12 +154,17 @@ def build_asgi_app( context_factory: ContextFactory | None = None, middleware: Sequence[SkillMiddleware] | None = None, streaming_responses: bool = False, + stateless_http: bool = False, + session_idle_timeout: float | None = 1800.0, + max_active_sessions: int | None = None, enable_dns_rebinding_protection: bool | None = None, max_request_size: int | None = None, validation: ValidationHookConfig | None = DEFAULT_VALIDATION, discovery_base_url: str | None = None, pre_validation_hooks: PreValidationHooks | None = None, response_enhancer: ResponseEnhancer | None = None, + on_startup: Sequence[LifespanHook] | None = None, + on_shutdown: Sequence[LifespanHook] | None = None, **factory_kwargs: Any, ) -> Any: """Build a Starlette ASGI app for in-process integration tests. @@ -219,6 +230,15 @@ def build_asgi_app( every tool dispatch. Forwarded to :func:`create_mcp_server`. :param streaming_responses: Forwarded to :func:`create_mcp_server`. Default ``False``. + :param stateless_http: Forwarded to :func:`create_mcp_server` for + ``transport="mcp"`` and to the production composition path for + ``transport="both"``. Ignored by ``transport="a2a"``. + :param session_idle_timeout: Idle reap deadline for stateful MCP + sessions. Defaults to 1800 seconds. Forwarded for ``"mcp"`` and + ``"both"``; ignored by ``"a2a"``. + :param max_active_sessions: Optional cap for active stateful MCP + sessions. Forwarded for ``"mcp"`` and ``"both"``; ignored by + ``"a2a"``. :param enable_dns_rebinding_protection: Forwarded to :func:`create_mcp_server`. ``None`` → FastMCP default. :param max_request_size: Request body size cap in bytes. ``None`` → @@ -244,6 +264,10 @@ def build_asgi_app( :func:`create_mcp_server`, so in-process tests exercise the same enhancer wiring your production :func:`serve` call uses. ``None`` → no enhancer (default). + :param on_startup: Async zero-argument hooks run after the MCP and A2A + framework lifespans start. Requires ``transport="both"``. + :param on_shutdown: Async zero-argument hooks run before the MCP and A2A + framework lifespans stop. Requires ``transport="both"``. :param factory_kwargs: Forwarded to :func:`create_adcp_server_from_platform`. Accepted keys: ``executor``, ``registry``, ``webhook_sender``, @@ -257,6 +281,27 @@ def build_asgi_app( """ if transport not in ("mcp", "a2a", "both"): raise ValueError(f"Unsupported transport {transport!r}; expected 'mcp', 'a2a', or 'both'.") + if (on_startup or on_shutdown) and transport != "both": + raise ValueError( + "on_startup / on_shutdown hooks require transport='both', " + f"got transport={transport!r}." + ) + if transport == "a2a": + ignored_session_settings = [] + if stateless_http: + ignored_session_settings.append("stateless_http") + if session_idle_timeout != 1800.0: + ignored_session_settings.append("session_idle_timeout") + if max_active_sessions is not None: + ignored_session_settings.append("max_active_sessions") + if ignored_session_settings: + warnings.warn( + "build_asgi_app sets MCP-only session fields " + f"{sorted(ignored_session_settings)} but transport='a2a'. " + "These fields will be ignored.", + UserWarning, + stacklevel=2, + ) from adcp.decisioning.serve import create_adcp_server_from_platform from adcp.server.serve import ( @@ -291,6 +336,9 @@ def build_asgi_app( advertise_all=advertise_all, max_request_size=max_request_size, streaming_responses=streaming_responses, + stateless_http=stateless_http, + session_idle_timeout=session_idle_timeout, + max_active_sessions=max_active_sessions, validation=validation, pre_validation_hooks=pre_validation_hooks, response_enhancer=response_enhancer, @@ -299,6 +347,8 @@ def build_asgi_app( allowed_origins=allowed_origins, enable_dns_rebinding_protection=enable_dns_rebinding_protection, auth=auth, + on_startup=on_startup, + on_shutdown=on_shutdown, include_discovery=discovery_base_url is not None, ) return _apply_asgi_middleware(app, asgi_middleware) @@ -331,6 +381,9 @@ def build_asgi_app( context_factory=context_factory, middleware=middleware, streaming_responses=streaming_responses, + stateless_http=stateless_http, + session_idle_timeout=session_idle_timeout, + max_active_sessions=max_active_sessions, enable_dns_rebinding_protection=enable_dns_rebinding_protection, validation=validation, pre_validation_hooks=pre_validation_hooks, @@ -373,12 +426,17 @@ async def build_test_client( context_factory: ContextFactory | None = None, middleware: Sequence[SkillMiddleware] | None = None, streaming_responses: bool = False, + stateless_http: bool = False, + session_idle_timeout: float | None = 1800.0, + max_active_sessions: int | None = None, enable_dns_rebinding_protection: bool | None = None, max_request_size: int | None = None, validation: ValidationHookConfig | None = DEFAULT_VALIDATION, discovery_base_url: str | None = None, pre_validation_hooks: PreValidationHooks | None = None, response_enhancer: ResponseEnhancer | None = None, + on_startup: Sequence[LifespanHook] | None = None, + on_shutdown: Sequence[LifespanHook] | None = None, **factory_kwargs: Any, ) -> AsyncIterator[httpx.AsyncClient]: """Async context manager yielding an ``httpx.AsyncClient`` wired against @@ -427,6 +485,9 @@ async def build_test_client( :param context_factory: Forwarded to :func:`build_asgi_app`. :param middleware: Forwarded to :func:`build_asgi_app`. :param streaming_responses: Forwarded to :func:`build_asgi_app`. + :param stateless_http: Forwarded to :func:`build_asgi_app`. + :param session_idle_timeout: Forwarded to :func:`build_asgi_app`. + :param max_active_sessions: Forwarded to :func:`build_asgi_app`. :param enable_dns_rebinding_protection: Forwarded to :func:`build_asgi_app`. :param max_request_size: Forwarded to :func:`build_asgi_app`. @@ -442,6 +503,10 @@ async def build_test_client( :param response_enhancer: Forwarded to :func:`build_asgi_app`. Wire the same enhancer your production :func:`serve` call uses so in-process tests exercise the enhancer path. + :param on_startup: Forwarded to :func:`build_asgi_app`. Requires + ``transport="both"``. + :param on_shutdown: Forwarded to :func:`build_asgi_app`. Requires + ``transport="both"``. :param factory_kwargs: Forwarded to :func:`create_adcp_server_from_platform` via :func:`build_asgi_app` (executor, registry, webhook_sender, etc.). @@ -492,12 +557,17 @@ async def build_test_client( context_factory=context_factory, middleware=middleware, streaming_responses=streaming_responses, + stateless_http=stateless_http, + session_idle_timeout=session_idle_timeout, + max_active_sessions=max_active_sessions, enable_dns_rebinding_protection=enable_dns_rebinding_protection, max_request_size=max_request_size, validation=validation, discovery_base_url=discovery_base_url, pre_validation_hooks=pre_validation_hooks, response_enhancer=response_enhancer, + on_startup=on_startup, + on_shutdown=on_shutdown, **factory_kwargs, ) async with LifespanManager(app): diff --git a/src/adcp/types/__init__.py b/src/adcp/types/__init__.py index f2f29c31c..c38dd1851 100644 --- a/src/adcp/types/__init__.py +++ b/src/adcp/types/__init__.py @@ -716,6 +716,7 @@ "UpdateContentStandardsSuccessResponse", "UpdateMediaBuyErrorResponse", "LegacyUpdateMediaBuyErrorResponse", + "LegacyUpdateMediaBuySuccessResponse", "UpdateMediaBuyResponse1", "UpdateMediaBuyResponse3", "UpdateMediaBuyPackagesRequest", @@ -845,6 +846,8 @@ "SignalCoverageForecast", "SignalCoverageRange", "MissingMetric", + "PropertyIdentifier", + "ProductFilterCountry", # Cross-module name collision aliases (#911, Step 2) # Creative "DeliveryCreative", @@ -1456,6 +1459,7 @@ def __dir__() -> list[str]: LegacySyncCreativesRequest, LegacyUpdateMediaBuyErrorResponse, LegacyUpdateMediaBuyRequest, + LegacyUpdateMediaBuySuccessResponse, ListAccountsRequest, ListAccountsResponse, ListCollectionListsRequest, @@ -1567,6 +1571,7 @@ def __dir__() -> list[str]: ProductCard, ProductCardDetailed, ProductCatalog, + ProductFilterCountry, ProductFilters, ProductFormatDeclaration, ProductFormatSellerPreference, @@ -1574,6 +1579,7 @@ def __dir__() -> list[str]: Property, PropertyId, PropertyIdActivationKey, + PropertyIdentifier, PropertyIdentifierTypes, PropertyList, PropertyListChangedWebhook, diff --git a/src/adcp/types/_eager.py b/src/adcp/types/_eager.py index e07823ca4..9e0437f5d 100644 --- a/src/adcp/types/_eager.py +++ b/src/adcp/types/_eager.py @@ -599,6 +599,7 @@ JavascriptFormatGroupAsset, KeyValueActivationKey, LegacyUpdateMediaBuyErrorResponse, + LegacyUpdateMediaBuySuccessResponse, ListContentStandardsErrorResponse, ListContentStandardsResponse1, ListContentStandardsSuccessResponse, @@ -628,8 +629,10 @@ PreviewRenderingOrigin, PricingOption, ProductAllocation, + ProductFilterCountry, ProductFormatSellerPreference, PropertyId, + PropertyIdentifier, PropertyTag, Provenance, ProvenanceDeclaredBy, @@ -1681,6 +1684,7 @@ def __init__(self, *args: object, **kwargs: object) -> None: "UpdateFrequency", "UpdateMediaBuyErrorResponse", "LegacyUpdateMediaBuyErrorResponse", + "LegacyUpdateMediaBuySuccessResponse", "UpdateMediaBuyPackagesRequest", "UpdateMediaBuyPropertiesRequest", "UpdateMediaBuyRequest", @@ -1763,6 +1767,8 @@ def __init__(self, *args: object, **kwargs: object) -> None: "PercentOfMediaVendorPricingOption", "PerUnitVendorPricingOption", "ProductAllocation", + "ProductFilterCountry", + "PropertyIdentifier", "SignalCoverageForecast", "SignalCoverageRange", "TrustedMatch", diff --git a/src/adcp/types/aliases.py b/src/adcp/types/aliases.py index 1fd06de44..3865771c3 100644 --- a/src/adcp/types/aliases.py +++ b/src/adcp/types/aliases.py @@ -2039,6 +2039,12 @@ class UnknownGroupAsset(_BaseGroupAsset): from adcp.types.generated_poc.core.overlay import ( Unit as OverlayUnit, ) +from adcp.types.generated_poc.core.product_filters import ( + Country as ProductFilterCountry, +) +from adcp.types.generated_poc.core.property import ( + Identifier as PropertyIdentifier, +) from adcp.types.generated_poc.core.provenance import ( DeclaredBy as ProvenanceDeclaredBy, ) @@ -2099,6 +2105,9 @@ class UnknownGroupAsset(_BaseGroupAsset): from adcp.types.generated_poc.media_buy.sync_event_sources_response import ( Setup as SyncEventSourcesSetup, ) +from adcp.types.generated_poc.media_buy.update_media_buy_response import ( + UpdateMediaBuyResponse1 as LegacyUpdateMediaBuySuccessResponse, +) from adcp.types.generated_poc.protocol.get_adcp_capabilities_response import ( Account as CapabilitiesAccount, ) @@ -2266,6 +2275,8 @@ class UnknownGroupAsset(_BaseGroupAsset): "SignalCoverageForecast", "SignalCoverageRange", "MissingMetric", + "PropertyIdentifier", + "ProductFilterCountry", # Canonical-formats v2 surface (AdCP 3.1) "CanonicalAssetSource", "CanonicalCompositionModel", @@ -2435,6 +2446,7 @@ class UnknownGroupAsset(_BaseGroupAsset): "UpdateMediaBuySuccessResponse", "UpdateMediaBuyErrorResponse", "LegacyUpdateMediaBuyErrorResponse", + "LegacyUpdateMediaBuySuccessResponse", "UpdateMediaBuyResponse3", "UpdateMediaBuySubmittedResponse", # Validate content delivery responses diff --git a/src/adcp/types/versioned.py b/src/adcp/types/versioned.py index 50bc4a57d..59d882303 100644 --- a/src/adcp/types/versioned.py +++ b/src/adcp/types/versioned.py @@ -20,6 +20,7 @@ from types import GenericAlias from typing import Any, ClassVar, Literal, Union +from jsonschema.validators import validator_for from pydantic import ( BaseModel, ConfigDict, @@ -212,6 +213,17 @@ def _fallback_annotation( return primitive_types.get(schema_type, Any) if isinstance(schema_type, str) else Any +def _schema_admits_null(schema: Any, document: dict[str, Any]) -> bool: + """Return whether *schema* accepts JSON ``null`` in its root document. + + Delegating the probe to the schema document's own validator handles every + supported nullable spelling consistently, including ``type: null``, type + arrays, local references, and ``anyOf`` / ``oneOf`` alternatives. + """ + validator_class = validator_for(document) + return bool(validator_class(document).evolve(schema=schema).is_valid(None)) + + class VersionedSchemaModel(RootModel[dict[str, Any]]): """Dict-shaped Pydantic model that enforces one bundled schema version. @@ -305,6 +317,7 @@ class _VersionedExtensionModel(BaseModel): schema_tool_name: ClassVar[str] schema_direction: ClassVar[VersionedDirection] schema_document: ClassVar[dict[str, Any]] + _omit_none_fields: ClassVar[frozenset[str]] = frozenset() @model_validator(mode="before") @classmethod @@ -327,6 +340,14 @@ def _protocol_payload(self) -> dict[str, Any]: exclude_unset=True, ) + @model_validator(mode="after") + def _normalize_optional_none(self) -> _VersionedExtensionModel: + """Treat explicit ``None`` as omission when null is not on the wire.""" + for name in self._omit_none_fields: + if getattr(self, name, None) is None: + self.model_fields_set.discard(name) + return self + @model_validator(mode="after") def _validate_schema_document(self) -> _VersionedExtensionModel: validator = get_validator( @@ -451,11 +472,18 @@ class SellerListCreativesRequest(ListCreatives31): guaranteed_fields = set.intersection(*(required for _properties, required in shapes)) current_annotations = _current_model_annotations(model_name) fields: dict[str, Any] = {} + omit_none_fields = frozenset( + name + for name, field_schema in properties.items() + if name not in guaranteed_fields and not _schema_admits_null(field_schema, schema) + ) for name, field_schema in properties.items(): annotation = current_annotations.get( name, _fallback_annotation(field_schema, schema), ) + if name not in guaranteed_fields: + annotation = Union.__getitem__((annotation, type(None))) description = field_schema.get("description") if isinstance(field_schema, dict) else None if isinstance(field_schema, dict) and "default" in field_schema: default = Field( @@ -480,6 +508,7 @@ class SellerListCreativesRequest(ListCreatives31): model.schema_tool_name = tool_name model.schema_direction = direction model.schema_document = schema + model._omit_none_fields = omit_none_fields return model diff --git a/tests/fixtures/public_api_snapshot.json b/tests/fixtures/public_api_snapshot.json index 01364641a..5e2804fd3 100644 --- a/tests/fixtures/public_api_snapshot.json +++ b/tests/fixtures/public_api_snapshot.json @@ -982,6 +982,7 @@ "LegacySyncCreativesRequest", "LegacyUpdateMediaBuyErrorResponse", "LegacyUpdateMediaBuyRequest", + "LegacyUpdateMediaBuySuccessResponse", "ListAccountsRequest", "ListAccountsResponse", "ListCollectionListsRequest", @@ -1093,6 +1094,7 @@ "ProductCard", "ProductCardDetailed", "ProductCatalog", + "ProductFilterCountry", "ProductFilters", "ProductFormatDeclaration", "ProductFormatSellerPreference", @@ -1100,6 +1102,7 @@ "Property", "PropertyId", "PropertyIdActivationKey", + "PropertyIdentifier", "PropertyIdentifierTypes", "PropertyList", "PropertyListChangedWebhook", diff --git a/tests/test_collision_aliases.py b/tests/test_collision_aliases.py index a46d64a18..3335f7b7b 100644 --- a/tests/test_collision_aliases.py +++ b/tests/test_collision_aliases.py @@ -43,6 +43,13 @@ "media_buy.update_media_buy_response", "UpdateMediaBuyResponse2", ), + ("PropertyIdentifier", "core.property", "Identifier"), + ("ProductFilterCountry", "core.product_filters", "Country"), + ( + "LegacyUpdateMediaBuySuccessResponse", + "media_buy.update_media_buy_response", + "UpdateMediaBuyResponse1", + ), # Creative — ListCreativesCreative deliberately remains the legacy # class-shaped alias for subclass compatibility. The 3.1.8 split also # exposes ListCreativesCanonicalCreative and ListCreativesCreativeItem. diff --git a/tests/test_migrate_v3_to_v4.py b/tests/test_migrate_v3_to_v4.py index 6192fd0be..0bde06a55 100644 --- a/tests/test_migrate_v3_to_v4.py +++ b/tests/test_migrate_v3_to_v4.py @@ -220,8 +220,8 @@ def test_flags_generated_poc_multiple_symbols_one_line(tmp_path: Path) -> None: _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import " - "BrandReference, ContextObject, MediaBuyStatus\n", + "from adcp.types.generated_poc.core.vendor_pricing_option import " + "VendorPricingOption1, VendorPricingOption2\n", ) report = v3_to_v4.run(tmp_path, apply_changes=False) @@ -229,9 +229,8 @@ def test_flags_generated_poc_multiple_symbols_one_line(tmp_path: Path) -> None: private = [f for f in report.flagged if f.kind == "flag_private"] by_symbol = {f.before: f.after for f in private} assert by_symbol == { - "BrandReference": "adcp.types.BrandReference", - "ContextObject": "adcp.types.ContextObject", - "MediaBuyStatus": "adcp.types.MediaBuyStatus", + "VendorPricingOption1": "adcp.types.CpmVendorPricingOption", + "VendorPricingOption2": "adcp.types.PercentOfMediaVendorPricingOption", } @@ -700,6 +699,45 @@ def test_auto_apply_uses_source_scoped_semantic_alias(tmp_path: Path) -> None: compile(rewritten, str(path), "exec") +@pytest.mark.parametrize( + ("module", "symbol", "public_name"), + [ + ("core.creative_asset", "CreativeAsset", "LegacyCreativeAsset"), + ( + "core.product_format_declaration", + "ProductFormatDeclaration", + "LegacyProductFormatDeclaration", + ), + ("signals.get_signals_response", "Signal", "GetSignalsSignal"), + ("core.account", "GovernanceAgent", "CoreGovernanceAgent"), + ("core.creative_filters", "CreativeFilters", "LegacyCreativeFilters"), + ("creative.list_creatives_request", "Sort", "ListCreativesSort"), + ("core.property", "Identifier", "PropertyIdentifier"), + ("core.product_filters", "Country", "ProductFilterCountry"), + ( + "media_buy.update_media_buy_response", + "UpdateMediaBuyResponse1", + "LegacyUpdateMediaBuySuccessResponse", + ), + ], +) +def test_auto_apply_uses_beta5_source_scoped_aliases( + tmp_path: Path, + module: str, + symbol: str, + public_name: str, +) -> None: + path = _write( + tmp_path, + "code.py", + f"from adcp.types.generated_poc.{module} import {symbol}\n", + ) + + v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) + + assert path.read_text() == f"from adcp.types import {public_name} as {symbol}\n" + + def test_auto_apply_preserves_crlf_after_source_scoped_import(tmp_path: Path) -> None: path = tmp_path / "code.py" path.write_bytes( @@ -720,10 +758,14 @@ def test_auto_apply_does_not_guess_colliding_source_variant(tmp_path: Path) -> N path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.product_filters import TrustedMatch\n", + "from adcp.types.generated_poc.core.product_filters import ProductFilters\n", ) - v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) + report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) assert "adcp.types.generated_poc.core.product_filters" in path.read_text() + finding = next(f for f in report.flagged if f.before == "ProductFilters") + assert finding.hint is not None + assert finding.hint.startswith("SKIP: source") + assert "manual rewrite required" in finding.hint def test_auto_apply_rewrites_multi_symbol_all_known_line(tmp_path: Path) -> None: @@ -731,14 +773,17 @@ def test_auto_apply_rewrites_multi_symbol_all_known_line(tmp_path: Path) -> None path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import " - "BrandReference, ContextObject, MediaBuyStatus\n", + "from adcp.types.generated_poc.core.vendor_pricing_option import " + "VendorPricingOption1, VendorPricingOption2\n", ) v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) rewritten = path.read_text() assert "adcp.types.generated_poc" not in rewritten - assert "from adcp.types import BrandReference, ContextObject, MediaBuyStatus" in rewritten + assert ( + "from adcp.types import CpmVendorPricingOption as VendorPricingOption1, " + "PercentOfMediaVendorPricingOption as VendorPricingOption2" + ) in rewritten def test_auto_apply_preserves_as_alias(tmp_path: Path) -> None: @@ -762,7 +807,7 @@ def test_auto_apply_mixed_line_not_rewritten(tmp_path: Path) -> None: path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import BrandReference, Unknown\n", + "from adcp.types.generated_poc.core.brand_ref import BrandReference, Unknown\n", ) report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) @@ -842,26 +887,27 @@ def test_auto_apply_unknown_numbered_stays_flagged(tmp_path: Path) -> None: assert report.auto_applied == [] -def test_auto_apply_numbered_plus_known_symbol_same_line(tmp_path: Path) -> None: - """A line mixing a numbered asset (Assets81) with a known symbol - (ContextObject) must be fully auto-applied: both symbols resolved, - import path corrected, nothing left in flagged.""" +def test_auto_apply_multiple_numbered_symbols_same_line(tmp_path: Path) -> None: + """A line containing two mapped numbered assets is fully rewritten.""" path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import Assets81, ContextObject\n" "slot: Assets81\n", + "from adcp.types.generated_poc.core.format import Assets81, Assets82\n" + "video: Assets81\n" + "audio: Assets82\n", ) report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True) rewritten = path.read_text() assert "adcp.types.generated_poc" not in rewritten - assert "from adcp.types import VideoFormatAsset, ContextObject" in rewritten - assert "slot: VideoFormatAsset" in rewritten + assert "from adcp.types import VideoFormatAsset, AudioFormatAsset" in rewritten + assert "video: VideoFormatAsset" in rewritten + assert "audio: AudioFormatAsset" in rewritten assert any( f.before == "Assets81" and f.after == "VideoFormatAsset" for f in report.auto_applied ) - assert any(f.before == "ContextObject" for f in report.auto_applied) + assert any(f.before == "Assets82" for f in report.auto_applied) assert not any(f.kind == "flag_private" for f in report.flagged) @@ -907,7 +953,7 @@ def test_auto_apply_implies_apply(tmp_path: Path) -> None: tmp_path, "code.py", "from adcp.types import AudioAsset\n" - "from adcp.types.generated_poc.core.x import ContextObject\n", + "from adcp.types.generated_poc.core.context import ContextObject\n", ) v3_to_v4.main([str(tmp_path), "--auto-apply"]) @@ -925,7 +971,7 @@ def test_dry_run_with_auto_apply_does_not_write_files(tmp_path: Path) -> None: path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n" + "from adcp.types.generated_poc.core.context import ContextObject\n" "from adcp.types.generated_poc.core.format import Assets81\n", ) original = path.read_text() @@ -958,7 +1004,7 @@ def test_auto_apply_idempotent(tmp_path: Path) -> None: path = _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n" + "from adcp.types.generated_poc.core.context import ContextObject\n" "from adcp.types.generated_poc.core.format import Assets81\n", ) @@ -979,7 +1025,7 @@ def test_auto_apply_exits_nonzero_when_flag_removed_remain(tmp_path: Path) -> No _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n" + "from adcp.types.generated_poc.core.context import ContextObject\n" "from adcp import BrandManifest\n", ) rc = v3_to_v4.main([str(tmp_path), "--auto-apply"]) @@ -991,7 +1037,7 @@ def test_auto_apply_exits_zero_when_only_safe_findings(tmp_path: Path) -> None: _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n" + "from adcp.types.generated_poc.core.context import ContextObject\n" "from adcp.types.generated_poc.core.format import Assets81\n", ) rc = v3_to_v4.main([str(tmp_path), "--auto-apply"]) @@ -1010,7 +1056,7 @@ def test_auto_apply_text_report_has_safe_rewrites_section( _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n", + "from adcp.types.generated_poc.core.context import ContextObject\n", ) v3_to_v4.main([str(tmp_path), "--auto-apply"]) out = capsys.readouterr().out @@ -1024,7 +1070,7 @@ def test_auto_apply_json_report_has_auto_applied_array( _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n", + "from adcp.types.generated_poc.core.context import ContextObject\n", ) v3_to_v4.main([str(tmp_path), "--auto-apply", "--json"]) payload = json.loads(capsys.readouterr().out) @@ -1061,7 +1107,7 @@ def test_text_report_shows_tip_when_safe_findings_remain( _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n", + "from adcp.types.generated_poc.core.context import ContextObject\n", ) v3_to_v4.main([str(tmp_path)]) out = capsys.readouterr().out @@ -1076,7 +1122,7 @@ def test_text_report_no_tip_when_auto_apply_active( _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import ContextObject\n", + "from adcp.types.generated_poc.core.context import ContextObject\n", ) v3_to_v4.main([str(tmp_path), "--auto-apply"]) out = capsys.readouterr().out @@ -1095,7 +1141,7 @@ def test_mixed_line_unknown_symbol_not_silently_dropped(tmp_path: Path) -> None: _write( tmp_path, "code.py", - "from adcp.types.generated_poc.core.x import BrandReference, Unknown\n", + "from adcp.types.generated_poc.core.brand_ref import BrandReference, Unknown\n", ) report = v3_to_v4.run(tmp_path, apply_changes=False) diff --git a/tests/test_testing_decisioning.py b/tests/test_testing_decisioning.py index 9183b21ee..e6dc8511c 100644 --- a/tests/test_testing_decisioning.py +++ b/tests/test_testing_decisioning.py @@ -234,6 +234,85 @@ def test_build_asgi_app_forwards_streaming_responses() -> None: assert callable(app) +def test_build_asgi_app_forwards_mcp_session_settings() -> None: + """MCP session controls reach the same factory used by production.""" + from unittest.mock import patch + + from adcp.server.serve import create_mcp_server + + with patch("adcp.server.serve.create_mcp_server", wraps=create_mcp_server) as mocked: + app = build_asgi_app( + _SalesPlatformWithMethods(), + stateless_http=True, + session_idle_timeout=None, + max_active_sessions=25, + ) + + assert callable(app) + assert mocked.call_args.kwargs["stateless_http"] is True + assert mocked.call_args.kwargs["session_idle_timeout"] is None + assert mocked.call_args.kwargs["max_active_sessions"] == 25 + + +def test_build_asgi_app_both_forwards_lifespan_and_session_settings() -> None: + """The combined topology receives the full production-parity surface.""" + from unittest.mock import patch + + from adcp.server.serve import _build_mcp_and_a2a_app + + async def startup() -> None: + pass + + async def shutdown() -> None: + pass + + with patch( + "adcp.server.serve._build_mcp_and_a2a_app", + wraps=_build_mcp_and_a2a_app, + ) as mocked: + app = build_asgi_app( + _SalesPlatformWithMethods(), + transport="both", + stateless_http=True, + session_idle_timeout=None, + max_active_sessions=25, + on_startup=(startup,), + on_shutdown=(shutdown,), + ) + + assert callable(app) + assert mocked.call_args.kwargs["stateless_http"] is True + assert mocked.call_args.kwargs["session_idle_timeout"] is None + assert mocked.call_args.kwargs["max_active_sessions"] == 25 + assert mocked.call_args.kwargs["on_startup"] == (startup,) + assert mocked.call_args.kwargs["on_shutdown"] == (shutdown,) + + +@pytest.mark.parametrize("transport", ["mcp", "a2a"]) +def test_build_asgi_app_rejects_lifespan_hooks_for_single_transport( + transport: Literal["mcp", "a2a"], +) -> None: + async def startup() -> None: + pass + + with pytest.raises(ValueError, match="hooks require transport='both'"): + build_asgi_app( + _SalesPlatformWithMethods(), + transport=transport, + on_startup=(startup,), + ) + + +def test_build_asgi_app_warns_when_a2a_ignores_mcp_session_settings() -> None: + with pytest.warns(UserWarning, match="MCP-only session fields"): + app = build_asgi_app( + _SalesPlatformWithMethods(), + transport="a2a", + session_idle_timeout=None, + ) + assert callable(app) + + def test_build_asgi_app_forwards_max_request_size() -> None: """``max_request_size=`` is accepted — construction succeeds.""" platform = _SalesPlatformWithMethods() @@ -511,6 +590,26 @@ async def test_build_test_client_forwards_validation_none() -> None: assert client is not None +async def test_build_test_client_runs_production_lifespan_hooks() -> None: + events: list[str] = [] + + async def startup() -> None: + events.append("startup") + + async def shutdown() -> None: + events.append("shutdown") + + async with build_test_client( + _SalesPlatformWithMethods(), + transport="both", + on_startup=(startup,), + on_shutdown=(shutdown,), + ): + assert events == ["startup"] + + assert events == ["startup", "shutdown"] + + # ---- build_test_client ---- diff --git a/tests/test_version_scoped_models.py b/tests/test_version_scoped_models.py index 26231f7bb..96aeefbf0 100644 --- a/tests/test_version_scoped_models.py +++ b/tests/test_version_scoped_models.py @@ -124,6 +124,62 @@ def test_versioned_base_uses_current_nested_runtime_models() -> None: assert request.model_dump(mode="json")["filters"] == {"statuses": ["approved"]} +def test_versioned_base_omits_explicit_none_for_optional_non_nullable_fields() -> None: + base = make_versioned_base("3.1", "ListCreativesRequest") + + request = base(account=None, context=None, filters=None) + + assert request.account is None + payload = request.model_dump() + assert payload["include_assignments"] is True + assert payload["include_snapshot"] is False + assert {"account", "context", "filters"}.isdisjoint(payload) + assert request.model_fields_set.isdisjoint({"account", "context", "filters"}) + + +def test_versioned_base_preserves_none_when_schema_admits_null( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from jsonschema.validators import validator_for + + from adcp.types import versioned + + schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "required_nullable": {"type": ["string", "null"]}, + "optional_any_of": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + "optional_one_of": {"oneOf": [{"type": "string"}, {"type": "null"}]}, + "optional_bare_null": {"type": "null"}, + "optional_non_nullable": {"type": "string"}, + }, + "required": ["required_nullable"], + "additionalProperties": False, + } + validator = validator_for(schema)(schema) + monkeypatch.setattr(versioned, "get_portable_schema", lambda *args, **kwargs: schema) + monkeypatch.setattr(versioned, "get_validator", lambda *args, **kwargs: validator) + versioned.make_versioned_base.cache_clear() + + base = versioned.make_versioned_base("test", "SyntheticRequest") + request = base( + required_nullable=None, + optional_any_of=None, + optional_one_of=None, + optional_bare_null=None, + optional_non_nullable=None, + ) + + assert request.model_dump() == { + "required_nullable": None, + "optional_any_of": None, + "optional_one_of": None, + "optional_bare_null": None, + } + versioned.make_versioned_base.cache_clear() + + def test_versioned_base_emits_only_the_canonical_pinned_schema() -> None: base = make_versioned_base("3.1", "ListCreativesRequest") From 9a2f8b8f957323c03ba0c31edfad9d939de9f4d2 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 20 Aug 2026 10:18:27 +0200 Subject: [PATCH 2/2] test(testing): make server patches Python 3.10 compatible --- tests/test_testing_decisioning.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_testing_decisioning.py b/tests/test_testing_decisioning.py index e6dc8511c..383f98daa 100644 --- a/tests/test_testing_decisioning.py +++ b/tests/test_testing_decisioning.py @@ -236,11 +236,16 @@ def test_build_asgi_app_forwards_streaming_responses() -> None: def test_build_asgi_app_forwards_mcp_session_settings() -> None: """MCP session controls reach the same factory used by production.""" + import importlib from unittest.mock import patch - from adcp.server.serve import create_mcp_server + serve_module = importlib.import_module("adcp.server.serve") - with patch("adcp.server.serve.create_mcp_server", wraps=create_mcp_server) as mocked: + with patch.object( + serve_module, + "create_mcp_server", + wraps=serve_module.create_mcp_server, + ) as mocked: app = build_asgi_app( _SalesPlatformWithMethods(), stateless_http=True, @@ -256,9 +261,10 @@ def test_build_asgi_app_forwards_mcp_session_settings() -> None: def test_build_asgi_app_both_forwards_lifespan_and_session_settings() -> None: """The combined topology receives the full production-parity surface.""" + import importlib from unittest.mock import patch - from adcp.server.serve import _build_mcp_and_a2a_app + serve_module = importlib.import_module("adcp.server.serve") async def startup() -> None: pass @@ -266,9 +272,10 @@ async def startup() -> None: async def shutdown() -> None: pass - with patch( - "adcp.server.serve._build_mcp_and_a2a_app", - wraps=_build_mcp_and_a2a_app, + with patch.object( + serve_module, + "_build_mcp_and_a2a_app", + wraps=serve_module._build_mcp_and_a2a_app, ) as mocked: app = build_asgi_app( _SalesPlatformWithMethods(),