fix(decisioning): wrap pydantic.ValidationError from delegates as INVALID_REQUEST

src/adcp/decisioning/dispatch.py

@@ -70,7 +70,7 @@
 if TYPE_CHECKING:
     from collections.abc import Awaitable, Callable
 
-    from pydantic import BaseModel
+    from pydantic import BaseModel, ValidationError
 
     from adcp.decisioning.accounts import AccountStore
     from adcp.decisioning.context import AuthInfo, RequestContext
@@ -632,6 +632,47 @@ def _internal_error_details(exc: BaseException) -> dict[str, Any]:
     return details
 
 
+# ---------------------------------------------------------------------------
+# _validation_error_to_invalid_request — request-validation error wrapper
+# ---------------------------------------------------------------------------
+
+
+def _validation_error_to_invalid_request(method_name: str, exc: ValidationError) -> AdcpError:
+    """Convert a ``pydantic.ValidationError`` raised inside a platform method
+    to ``AdcpError('INVALID_REQUEST', recovery='correctable')``.
+
+    The generic ``except Exception`` handler wraps all unhandled exceptions as
+    ``INTERNAL_ERROR``. But a ``ValidationError`` from a platform delegate
+    almost always means the buyer supplied a request field that failed the
+    seller's stricter schema — semantically an ``INVALID_REQUEST`` the buyer
+    can correct. This matches the behaviour of
+    :func:`_coerce_params_to_platform_type` for the annotation-coercion path.
+
+    Uses :func:`adcp.types.error_narrowing.narrow_union_errors` to strip
+    discriminated-union noise from the ``details.validation_errors`` list.
+    Both ``narrow_union_errors`` and ``exc.errors()`` are part of stable
+    in-repo / Pydantic APIs respectively, so a failure here would be a
+    genuine bug worth surfacing rather than masking with a fallback.
+    """
+    from adcp.types.error_narrowing import narrow_union_errors
+
+    raw = exc.errors(include_input=False, include_context=False, include_url=False)
+    errors: list[Any] = list(narrow_union_errors(raw))
+    first: dict[str, Any] = dict(errors[0]) if errors else {}
+    field_path = ".".join(str(loc) for loc in first.get("loc", ()))
+    msg = first.get("msg", "validation failed")
+    return AdcpError(
+        "INVALID_REQUEST",
+        message=(
+            f"Request validation failed for {method_name!r}: {msg}"
+            + (f" (field: {field_path!r})" if field_path else "")
+        ),
+        field=field_path or None,
+        recovery="correctable",
+        details={"validation_errors": errors},
+    )
+
+
 # ---------------------------------------------------------------------------
 # validate_platform — server-boot fail-fast
 # ---------------------------------------------------------------------------
@@ -1294,6 +1335,10 @@ async def _invoke_platform_method(
         to release the consumption reservation so the buyer can retry.
         Hook errors are logged but never block exception propagation.
     """
+    # pydantic is a required dep; import here (not at module level) to mirror
+    # the lazy-import discipline used throughout this module.
+    from pydantic import ValidationError as _ValidationError  # noqa: PLC0415
+
     method = getattr(platform, method_name)
     # Re-validate through the platform method's own annotation when it's a
     # stricter subclass of the shim's already-deserialized type.  Skipped
@@ -1409,6 +1454,21 @@ async def _invoke_platform_method(
         if on_failure is not None:
             await _safe_on_failure_call(on_failure, wrapped, method_name)
         raise wrapped from exc
+    except _ValidationError as exc:
+        # A ValidationError that escaped the platform delegate is almost
+        # always the buyer sending a field that fails the seller's stricter
+        # request schema.  Surface it as INVALID_REQUEST (correctable) so
+        # the buyer knows the payload is fixable and gets the field path.
+        # Mirrors _coerce_params_to_platform_type for the annotation path.
+        logger.warning(
+            "pydantic.ValidationError in platform.%s — wrapping to INVALID_REQUEST",
+            method_name,
+            exc_info=True,
+        )
+        wrapped = _validation_error_to_invalid_request(method_name, exc)
+        if on_failure is not None:
+            await _safe_on_failure_call(on_failure, wrapped, method_name)
+        raise wrapped from exc
     except Exception as exc:
         # Wrap unexpected exceptions so the wire never sees a stack
         # trace. Adopter logs the original via observability hooks;

tests/test_decisioning_dispatch.py

@@ -784,14 +784,16 @@ async def get_products(self, req, ctx):
 async def test_invoke_validation_error_surfaces_narrowed_field_paths(
     executor: ThreadPoolExecutor,
 ) -> None:
-    """When the platform method raises ``pydantic.ValidationError``
-    directly — typically because the seller constructed an invalid
-    response model — the wire ``details`` MUST carry the narrowed
-    field-path list so the buyer agent sees what failed (Stability AI
-    Emma P1: pre-fix wire said "see details for cause" with empty
-    details). Field paths are pulled from
-    ``ValidationError.errors()`` and run through
-    ``narrow_union_errors`` to filter discriminated-union noise."""
+    """Any ``pydantic.ValidationError`` escaping a platform method body
+    is now wrapped as ``INVALID_REQUEST`` (correctable) with structured
+    field paths in ``details.validation_errors`` — not ``INTERNAL_ERROR``.
+
+    The framework cannot distinguish a buyer-request validation failure
+    from a seller-response construction bug at this catch boundary; treating
+    both as ``INVALID_REQUEST`` is consistent with ``_coerce_params_to_platform_type``
+    and gives buyers the field information they need to fix the request.
+    Field paths are narrowed via ``narrow_union_errors`` to strip
+    discriminated-union noise."""
     from pydantic import BaseModel
 
     class _ResponseModel(BaseModel):
@@ -804,10 +806,6 @@ class _CrashingPlatform(DecisioningPlatform):
         accounts = SingletonAccounts(account_id="x")
 
         async def get_products(self, req, ctx):
-            # Seller-side bug: building a response with missing fields.
-            # Realistic shape: an adopter calling
-            # CreativeManifest(...) with a missing required ``url`` on
-            # ImageContent.
             _ResponseModel.model_validate({"creative_id": "cr-1"})
 
     ctx = _build_request_context(ToolContext(), Account(id="x"), None)
@@ -820,17 +818,65 @@ async def get_products(self, req, ctx):
             executor=executor,
             registry=InMemoryTaskRegistry(),
         )
-    assert exc_info.value.code == "INTERNAL_ERROR"
-    # ``caused_by`` still surfaces the exception class for triage.
-    assert exc_info.value.details["caused_by"]["type"] == "ValidationError"
-    # NEW: ``validation_errors`` is populated with structured field
-    # paths so the buyer agent (and the seller's wire log) see the
-    # actual missing fields.
+    assert exc_info.value.code == "INVALID_REQUEST"
+    assert exc_info.value.recovery == "correctable"
+    # ``validation_errors`` carries structured field paths.
     validation_errors = exc_info.value.details["validation_errors"]
     missing_fields = {err["loc"][-1] for err in validation_errors if err["type"] == "missing"}
     assert "width" in missing_fields and "height" in missing_fields
 
 
+@pytest.mark.asyncio
+async def test_invoke_validation_error_from_manual_model_validate_is_invalid_request(
+    executor: ThreadPoolExecutor,
+) -> None:
+    """Platform method manually calls ``RequestModel.model_validate()``
+    with a stricter subclass (``extra='forbid'``) and the buyer sends an
+    extra field — the resulting ValidationError must surface as
+    ``INVALID_REQUEST`` with ``recovery='correctable'`` and a populated
+    ``field`` pointing at the first failing loc (issue #652)."""
+    from pydantic import BaseModel, ConfigDict
+
+    class _StrictRequest(BaseModel):
+        model_config = ConfigDict(extra="forbid")
+        product_id: str
+
+    class _LooseRequest(BaseModel):
+        product_id: str
+        extra_field: str
+
+    class _Platform(DecisioningPlatform):
+        capabilities = DecisioningCapabilities()
+        accounts = SingletonAccounts(account_id="x")
+
+        async def get_products(self, req, ctx):
+            # Seller re-validates with stricter schema; buyer sent extra_field.
+            _StrictRequest.model_validate(req.model_dump())
+            return {}
+
+    ctx = _build_request_context(ToolContext(), Account(id="x"), None)
+    req = _LooseRequest(product_id="p1", extra_field="unexpected")
+
+    with pytest.raises(AdcpError) as exc_info:
+        await _invoke_platform_method(
+            _Platform(),
+            "get_products",
+            req,
+            ctx,
+            executor=executor,
+            registry=InMemoryTaskRegistry(),
+        )
+    err = exc_info.value
+    assert err.code == "INVALID_REQUEST"
+    assert err.recovery == "correctable"
+    # field should reference the rejected extra field
+    assert err.field is not None
+    assert "extra_field" in (err.field or "")
+    # full error list in details
+    assert "validation_errors" in err.details
+    assert any("extra_field" in str(e.get("loc", "")) for e in err.details["validation_errors"])
+
+
 @pytest.mark.asyncio
 async def test_invoke_arg_projector_signature_drift_projects_invalid_request(
     executor: ThreadPoolExecutor,