diff --git a/src/adcp/server/mcp_tools.py b/src/adcp/server/mcp_tools.py index 963a69429..f5bb5227d 100644 --- a/src/adcp/server/mcp_tools.py +++ b/src/adcp/server/mcp_tools.py @@ -1379,6 +1379,205 @@ def _apply_pydantic_schemas() -> None: _apply_pydantic_schemas() +def _generate_pydantic_output_schemas() -> dict[str, dict[str, Any]]: + """Generate JSON output schemas from Pydantic response models. + + Maps tool names to their corresponding response Pydantic types and + generates JSON Schema via ``model_json_schema()`` / ``TypeAdapter``. + + Unlike the input schema generator, this does **not** skip ``anyOf`` + union types — ``outputSchema`` on ``tools/list`` is informational + and ``anyOf`` is valid there (clients advertise what they can return, + not what they require as input). + + The result is applied to ``ADCP_TOOL_DEFINITIONS`` at import time by + :func:`_apply_pydantic_output_schemas`. + """ + try: + from pydantic import TypeAdapter + + from adcp.types import ( + AcquireRightsResponse, + ActivateSignalResponse, + BuildCreativeResponse, + CalibrateContentResponse, + CheckGovernanceResponse, + ComplyTestControllerResponse, + ContextMatchResponse, + CreateCollectionListResponse, + CreateContentStandardsResponse, + CreateMediaBuyResponse, + CreatePropertyListResponse, + DeleteCollectionListResponse, + DeletePropertyListResponse, + GetAccountFinancialsResponse, + GetAdcpCapabilitiesResponse, + GetBrandIdentityResponse, + GetCollectionListResponse, + GetContentStandardsResponse, + GetCreativeDeliveryResponse, + GetCreativeFeaturesResponse, + GetMediaBuyArtifactsResponse, + GetMediaBuyDeliveryResponse, + GetMediaBuysResponse, + GetPlanAuditLogsResponse, + GetProductsResponse, + GetPropertyListResponse, + GetRightsResponse, + GetSignalsResponse, + IdentityMatchResponse, + ListAccountsResponse, + ListCollectionListsResponse, + ListContentStandardsResponse, + ListCreativeFormatsResponse, + ListCreativesResponse, + ListPropertyListsResponse, + LogEventResponse, + PreviewCreativeResponse, + ProvidePerformanceFeedbackResponse, + ReportPlanOutcomeResponse, + ReportUsageResponse, + SiGetOfferingResponse, + SiInitiateSessionResponse, + SiSendMessageResponse, + SiTerminateSessionResponse, + SyncAccountsResponse, + SyncAudiencesResponse, + SyncCatalogsResponse, + SyncCreativesResponse, + SyncEventSourcesResponse, + SyncGovernanceResponse, + SyncPlansResponse, + UpdateCollectionListResponse, + UpdateContentStandardsResponse, + UpdateMediaBuyResponse, + UpdatePropertyListResponse, + UpdateRightsResponse, + ValidateContentDeliveryResponse, + ) + except ImportError: + return {} + + _tool_to_response: dict[str, Any] = { + # Catalog + "get_products": GetProductsResponse, + "list_creative_formats": ListCreativeFormatsResponse, + # Creative + "sync_creatives": SyncCreativesResponse, + "list_creatives": ListCreativesResponse, + "build_creative": BuildCreativeResponse, + "preview_creative": PreviewCreativeResponse, + "get_creative_delivery": GetCreativeDeliveryResponse, + # Media Buy + "create_media_buy": CreateMediaBuyResponse, + "update_media_buy": UpdateMediaBuyResponse, + "get_media_buy_delivery": GetMediaBuyDeliveryResponse, + "get_media_buys": GetMediaBuysResponse, + # Signals + "get_signals": GetSignalsResponse, + "activate_signal": ActivateSignalResponse, + # Account + "list_accounts": ListAccountsResponse, + "sync_accounts": SyncAccountsResponse, + "get_account_financials": GetAccountFinancialsResponse, + "report_usage": ReportUsageResponse, + # Events & Catalogs + "log_event": LogEventResponse, + "sync_event_sources": SyncEventSourcesResponse, + "sync_audiences": SyncAudiencesResponse, + "sync_catalogs": SyncCatalogsResponse, + "sync_governance": SyncGovernanceResponse, + # Feedback + "provide_performance_feedback": ProvidePerformanceFeedbackResponse, + # Protocol Discovery + "get_adcp_capabilities": GetAdcpCapabilitiesResponse, + # Compliance + "comply_test_controller": ComplyTestControllerResponse, + # Content Standards + "create_content_standards": CreateContentStandardsResponse, + "get_content_standards": GetContentStandardsResponse, + "list_content_standards": ListContentStandardsResponse, + "update_content_standards": UpdateContentStandardsResponse, + "calibrate_content": CalibrateContentResponse, + "validate_content_delivery": ValidateContentDeliveryResponse, + "get_media_buy_artifacts": GetMediaBuyArtifactsResponse, + # Governance + "get_creative_features": GetCreativeFeaturesResponse, + "sync_plans": SyncPlansResponse, + "check_governance": CheckGovernanceResponse, + "report_plan_outcome": ReportPlanOutcomeResponse, + "get_plan_audit_logs": GetPlanAuditLogsResponse, + # Property Lists + "create_property_list": CreatePropertyListResponse, + "get_property_list": GetPropertyListResponse, + "list_property_lists": ListPropertyListsResponse, + "update_property_list": UpdatePropertyListResponse, + "delete_property_list": DeletePropertyListResponse, + # Collection Lists + "create_collection_list": CreateCollectionListResponse, + "get_collection_list": GetCollectionListResponse, + "list_collection_lists": ListCollectionListsResponse, + "update_collection_list": UpdateCollectionListResponse, + "delete_collection_list": DeleteCollectionListResponse, + # Sponsored Intelligence + "si_get_offering": SiGetOfferingResponse, + "si_initiate_session": SiInitiateSessionResponse, + "si_send_message": SiSendMessageResponse, + "si_terminate_session": SiTerminateSessionResponse, + # Brand + "get_brand_identity": GetBrandIdentityResponse, + "get_rights": GetRightsResponse, + "acquire_rights": AcquireRightsResponse, + "update_rights": UpdateRightsResponse, + # TMP + "context_match": ContextMatchResponse, + "identity_match": IdentityMatchResponse, + } + + schemas: dict[str, dict[str, Any]] = {} + for tool_name, response_type in _tool_to_response.items(): + try: + if isinstance(response_type, type) and hasattr(response_type, "model_json_schema"): + schema = response_type.model_json_schema() + else: + adapter = TypeAdapter(response_type) + schema = adapter.json_schema() + + schema.pop("title", None) + + # Inline every $ref into its $defs body — same rationale as for + # inputSchema (MCP clients that don't resolve $ref see empty + # schemas). For outputSchema, anyOf at root is valid (union + # responses advertise what a tool may return), so we don't skip + # union types here. + schema = _inline_refs(schema) + + schemas[tool_name] = schema + except Exception: + logger.debug( + "Pydantic output schema generation failed for %s, skipping", + tool_name, + exc_info=True, + ) + + return schemas + + +# Generate output schemas once at import time +_PYDANTIC_OUTPUT_SCHEMAS = _generate_pydantic_output_schemas() + + +def _apply_pydantic_output_schemas() -> None: + """Write Pydantic-generated outputSchemas into ADCP_TOOL_DEFINITIONS.""" + for tool_def in ADCP_TOOL_DEFINITIONS: + name = tool_def["name"] + if name in _PYDANTIC_OUTPUT_SCHEMAS: + tool_def["outputSchema"] = _PYDANTIC_OUTPUT_SCHEMAS[name] + + +_apply_pydantic_output_schemas() + + def _is_sdk_base_class(cls_name: str) -> bool: """True when ``cls_name`` is registered in ``_HANDLER_TOOLS``. diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index 093bc7b37..1f4479326 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -1283,6 +1283,7 @@ def _register_handler_tools( continue description = tool_def.get("description", "") input_schema = tool_def.get("inputSchema", {"type": "object", "properties": {}}) + output_schema = tool_def.get("outputSchema") caller = create_tool_caller(handler, tool_name) _register_tool( mcp, @@ -1290,6 +1291,7 @@ def _register_handler_tools( description, input_schema, caller, + output_schema=output_schema, context_factory=context_factory, middleware=middleware_tuple, ) @@ -1310,15 +1312,23 @@ def _register_tool( input_schema: dict[str, Any], caller: Callable[..., Any], *, + output_schema: dict[str, Any] | None = None, context_factory: ContextFactory | None = None, middleware: tuple[SkillMiddleware, ...] = (), ) -> None: """Register a single ADCP tool on a FastMCP server. Creates a Tool with a permissive arg model that accepts any fields, - then overrides the advertised schema with the Pydantic-generated one. - This ensures MCP clients see the correct schema while the handler + then overrides the advertised schemas with the Pydantic-generated ones. + This ensures MCP clients see the correct schemas while the handler receives all parameters as a plain dict. + + ``output_schema``, when provided, replaces the generic + ``dict[str, Any]``-derived schema that FastMCP infers from the ``fn`` + return type. The per-tool Pydantic response schema is the one that + appears in ``tools/list``; the generic ``output_model`` is preserved + for the runtime ``structuredContent`` path (FastMCP validates the dict + the handler returns via the permissive model, not the spec schema). """ from mcp.server.fastmcp.tools import Tool from mcp.server.fastmcp.utilities.func_metadata import ArgModelBase, FuncMetadata @@ -1410,7 +1420,10 @@ async def _call_handler() -> Any: # Override fn_metadata with a permissive model that passes through # all fields as individual kwargs (instead of wrapping in a "kwargs" field). - # Keep the output_schema/output_model so structuredContent is populated. + # Use the per-tool output_schema for tools/list advertisement; keep the + # FastMCP-derived output_model (generic dict acceptor) for the runtime + # structuredContent path so FuncMetadata.convert_result can validate the + # handler's dict return without applying the stricter spec schema at call time. class _AdcpArgs(ArgModelBase): model_config = ConfigDict(extra="allow") @@ -1422,9 +1435,12 @@ def model_dump_one_level(self) -> dict[str, Any]: result.update(self.model_extra) return result + effective_output_schema = ( + output_schema if output_schema is not None else tool.fn_metadata.output_schema + ) tool.fn_metadata = FuncMetadata( arg_model=_AdcpArgs, - output_schema=tool.fn_metadata.output_schema, + output_schema=effective_output_schema, output_model=tool.fn_metadata.output_model, wrap_output=False, ) diff --git a/src/adcp/server/test_controller.py b/src/adcp/server/test_controller.py index 5a588cf30..7779dda46 100644 --- a/src/adcp/server/test_controller.py +++ b/src/adcp/server/test_controller.py @@ -729,10 +729,14 @@ async def comply_test_controller(**kwargs: Any) -> dict[str, Any]: ) return await _handle_test_controller(store, kwargs, context=context) + # structured_output=True gives FastMCP a generic dict output_model so + # FuncMetadata.convert_result can populate structuredContent at call time. + # We'll replace output_schema below with the spec-accurate response schema. tool = Tool.from_function( comply_test_controller, name="comply_test_controller", description="Compliance test controller. Sandbox only, not for production use.", + structured_output=True, ) # Override schema with the proper comply_test_controller inputSchema. @@ -752,7 +756,16 @@ async def comply_test_controller(**kwargs: Any) -> dict[str, Any]: "required": ["scenario"], } - # Override fn_metadata with a permissive model + # Look up the spec-accurate outputSchema from ADCP_TOOL_DEFINITIONS. + from adcp.server.mcp_tools import ADCP_TOOL_DEFINITIONS as _TOOL_DEFS + + _comply_def = next( + (t for t in _TOOL_DEFS if t["name"] == "comply_test_controller"), {} + ) + _comply_output_schema = _comply_def.get("outputSchema") + + # Override fn_metadata with a permissive model; use the per-tool output_schema + # for tools/list advertisement while keeping the generic output_model for runtime. class _ControllerArgs(ArgModelBase): model_config = ConfigDict(extra="allow") @@ -764,9 +777,13 @@ def model_dump_one_level(self) -> dict[str, Any]: result.update(self.model_extra) return result + _fallback = tool.fn_metadata.output_schema + effective_output_schema = ( + _comply_output_schema if _comply_output_schema is not None else _fallback + ) tool.fn_metadata = FuncMetadata( arg_model=_ControllerArgs, - output_schema=tool.fn_metadata.output_schema, + output_schema=effective_output_schema, output_model=tool.fn_metadata.output_model, wrap_output=tool.fn_metadata.wrap_output, ) diff --git a/tests/test_mcp_schema_drift.py b/tests/test_mcp_schema_drift.py index 38aaf166f..4b4ec3a93 100644 --- a/tests/test_mcp_schema_drift.py +++ b/tests/test_mcp_schema_drift.py @@ -1,13 +1,14 @@ -"""Drift tests for MCP tool inputSchema generation. +"""Drift tests for MCP tool inputSchema and outputSchema generation. -The MCP tool registry exposes ``inputSchema`` for every ADCP tool via -``tools/list``. These schemas are auto-generated from the corresponding -Pydantic request models in ``adcp.types`` at import time -(:func:`adcp.server.mcp_tools._generate_pydantic_schemas`). +The MCP tool registry exposes ``inputSchema`` and ``outputSchema`` for every +ADCP tool via ``tools/list``. These schemas are auto-generated from the +corresponding Pydantic request/response models in ``adcp.types`` at import +time (:func:`adcp.server.mcp_tools._generate_pydantic_schemas` / +:func:`adcp.server.mcp_tools._generate_pydantic_output_schemas`). -This module protects the generation path from regressions: +This module protects both generation paths from regressions: -1. Every tool must resolve to a Pydantic-generated schema. If a new tool +1. Every tool must resolve to a Pydantic-generated inputSchema. If a new tool is added to ``ADCP_TOOL_DEFINITIONS`` without a mapping in ``_tool_to_request``, the tool would silently ship a hand-crafted stub schema again — the drift this whole mechanism exists to prevent. @@ -17,6 +18,10 @@ or a model gains/drops a field, this test fails on the affected tool. 3. The schema must advertise the model's required fields so agents constructing payloads via ``tools/list`` see accurate constraints. +4. Every tool must also resolve to a Pydantic-generated outputSchema + (response model mapping). outputSchema may use ``anyOf`` for union + response types — that is valid MCP contract for what a tool returns. +5. Each tool's ``outputSchema`` must match fresh generation — no drift. """ from __future__ import annotations @@ -24,8 +29,10 @@ import json from adcp.server.mcp_tools import ( + _PYDANTIC_OUTPUT_SCHEMAS, _PYDANTIC_SCHEMAS, ADCP_TOOL_DEFINITIONS, + _generate_pydantic_output_schemas, _generate_pydantic_schemas, _inline_refs, ) @@ -481,3 +488,100 @@ def test_inlined_schema_still_validates_real_request() -> None: # And Pydantic still accepts it — both sides of the equivalence. GetProductsRequest.model_validate(payload) + + +# --------------------------------------------------------------------------- +# outputSchema — parity with TS SDK (issue #386) +# --------------------------------------------------------------------------- + + +def test_every_tool_has_pydantic_generated_output_schema() -> None: + """Every ADCP tool must map to a Pydantic response model so tools/list + carries outputSchema. If a new tool is added to ADCP_TOOL_DEFINITIONS + without a mapping in _tool_to_response, it silently ships with no + outputSchema — add it to the generator.""" + tool_names = {t["name"] for t in ADCP_TOOL_DEFINITIONS} + missing = tool_names - set(_PYDANTIC_OUTPUT_SCHEMAS.keys()) + assert not missing, ( + "Tools missing from Pydantic output schema generator — they would ship " + "with no outputSchema on tools/list:\n" + + "\n".join(f" - {name}" for name in sorted(missing)) + + "\n\nAdd each tool to ``_tool_to_response`` in " + "``adcp/server/mcp_tools.py``, mapped to its ``Response`` model." + ) + + +def test_output_schemas_applied_to_tool_definitions() -> None: + """Every tool in ADCP_TOOL_DEFINITIONS must have an outputSchema key + after import-time application.""" + missing = [t["name"] for t in ADCP_TOOL_DEFINITIONS if "outputSchema" not in t] + assert not missing, ( + "Tools missing outputSchema in ADCP_TOOL_DEFINITIONS:\n" + + "\n".join(f" - {name}" for name in missing) + + "\n\nCheck _apply_pydantic_output_schemas() runs at import time." + ) + + +def test_output_schemas_match_pydantic_generation() -> None: + """tools/list outputSchemas must byte-match fresh generation — no silent drift.""" + fresh = _generate_pydantic_output_schemas() + mismatches: list[str] = [] + for tool in ADCP_TOOL_DEFINITIONS: + name = tool["name"] + if name not in fresh: + continue + expected = fresh[name] + actual = tool.get("outputSchema") + if json.dumps(actual, sort_keys=True) != json.dumps(expected, sort_keys=True): + mismatches.append(name) + + assert not mismatches, ( + "ADCP_TOOL_DEFINITIONS has stale outputSchemas — " + "`_apply_pydantic_output_schemas()` must run at import time:\n" + + "\n".join(f" - {name}" for name in mismatches) + ) + + +def test_no_dollar_ref_in_any_output_schema() -> None: + """Every tool's outputSchema must be ``$ref``-free after inlining. + Unlike inputSchema, anyOf at root is permitted (union response types + advertise what a tool may return). But unresolved $ref nodes would + leave clients unable to resolve the schema.""" + for tool in ADCP_TOOL_DEFINITIONS: + schema = tool.get("outputSchema") + if schema is None: + continue + serialized = json.dumps(schema) + assert '"$ref"' not in serialized, ( + f"tool {tool['name']!r} outputSchema contains unresolved $ref. " + "Check _inline_refs in adcp.server.mcp_tools." + ) + + +def test_output_schema_spot_check_known_shapes() -> None: + """Spot-check that representative tools carry the expected outputSchema + shape so structural changes to response models surface here first.""" + tool_schemas = {t["name"]: t.get("outputSchema") for t in ADCP_TOOL_DEFINITIONS} + + # get_products: simple model — must advertise the top-level products array + gp = tool_schemas["get_products"] + assert gp is not None, "get_products must have outputSchema" + assert "products" in gp.get("properties", {}), ( + "get_products outputSchema must include the 'products' field" + ) + + # create_media_buy: union response (success | error) — anyOf at root + cmb = tool_schemas["create_media_buy"] + assert cmb is not None, "create_media_buy must have outputSchema" + assert "anyOf" in cmb, ( + "create_media_buy outputSchema should be anyOf (union of success/error variants)" + ) + + # get_adcp_capabilities: simple model — must advertise adcp + supported_protocols + gac = tool_schemas["get_adcp_capabilities"] + assert gac is not None, "get_adcp_capabilities must have outputSchema" + props = gac.get("properties", {}) + assert "adcp" in props, "get_adcp_capabilities outputSchema must include 'adcp'" + assert "supported_protocols" in props, ( + "get_adcp_capabilities outputSchema must include 'supported_protocols'" + )