Raise fastmcp.ValidationError for invalid tool arguments (#4392)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>

.claude/skills/review-pr/SKILL.md

@@ -94,6 +94,15 @@ After evaluating comments:
 
 Codex sometimes re-posts old comments that reference code you've already fixed (they appear on the old commit's diff). These are stale — verify the fix is in the latest commit and reply noting the fix is already in place.
 
+## Labels — never apply or invent them
+
+**Do not apply labels to PRs or issues programmatically, and never create new ones.** Labeling is the maintainer's call (and is often automated). Two hard rules:
+
+- **Never invent a label.** GitHub's "add labels" API *auto-creates* any label name that doesn't already exist — so a typo or a guessed name silently pollutes the repo's label list with a stray, uncolored duplicate. Adding `breaking` (which does not exist) creates it alongside the real `breaking change` label.
+- **Use only labels that already exist.** If you genuinely need to confirm a label, look it up first (`get_label` / the repo's label list) and match the exact name. The canonical names here are specific — e.g. the breaking-change label is **`breaking change`**, not `breaking`; enhancements is **`enhancements`**, features is **`features`**, bugs is **`bugs`**.
+
+When a change warrants a label (e.g. it's breaking), **say so in the PR body and let the maintainer apply the label** rather than applying it yourself. There is no MCP tool to delete a label, so a mistaken creation can only be cleaned up by hand in repo settings — the cost of guessing is high and one-directional.
+
 ## When a PR is ready
 
 A PR is ready for human review when:

CLAUDE.md

@@ -60,7 +60,7 @@ When modifying MCP functionality, changes typically need to be applied across al
 
 - Prek hooks are required (run automatically on commits)
 - Never amend commits to fix prek failures
-- Apply PR labels: bugs/breaking/enhancements/features
+- Never apply labels manually or invent new ones — the GitHub API auto-creates any unknown label name, polluting the repo's label list. Note the appropriate label in the PR body and let the maintainer/automation apply it. Canonical names: `bugs`, `breaking change`, `enhancements`, `features` (it's `breaking change`, not `breaking`). See the review-pr skill.
 - Improvements = enhancements (not features) unless specified
 - **NEVER** force-push on collaborative repos
 - **ALWAYS** run prek before PRs

fastmcp_slim/fastmcp/server/server.py

@@ -54,6 +54,7 @@ from fastmcp.exceptions import (
     PromptError,
     ResourceError,
     ToolError,
+    ValidationError,
 )
 from fastmcp.mcp_config import MCPConfig
 from fastmcp.prompts import Prompt
@@ -1280,13 +1281,29 @@ class FastMCP(
                     task_meta = replace(task_meta, fn_key=tool.key)
                 try:
                     return await tool._run(arguments or {}, task_meta=task_meta)
+                except ValidationError as e:
+                    # Argument-validation failure (a bad call). FunctionTool
+                    # converts pydantic's call-validation error into fastmcp's
+                    # ValidationError (see #4128) so it can be filtered as a
+                    # client error. Log the underlying detail without a URL or
+                    # traceback, matching the previous pydantic-error logging.
+                    cause = e.__cause__
+                    detail = (
+                        cause.errors(include_url=False)
+                        if isinstance(cause, PydanticValidationError)
+                        else str(e)
+                    )
+                    logger.warning("Invalid arguments for tool %r: %s", name, detail)
+                    raise
                 except FastMCPError as e:
                     logger.log(
                         e.log_level, f"Error calling tool {name!r}", exc_info=False
                     )
                     raise
                 except PydanticValidationError as e:
-                    # fastmcp's own ValidationError is a FastMCPError, already handled above.
+                    # A pydantic error that is NOT an argument-validation failure
+                    # (e.g. raised by a non-FunctionTool's own validation). Kept
+                    # for backward compatibility.
                     logger.warning(
                         "Invalid arguments for tool %r: %s",
                         name,

fastmcp_slim/fastmcp/tools/function_tool.py

@@ -4,9 +4,11 @@ from __future__ import annotations
 
 import functools
 import inspect
+import logging
 import warnings
 from collections.abc import Callable
 from dataclasses import dataclass, field
+from functools import lru_cache
 from types import MethodType
 from typing import (
     TYPE_CHECKING,
@@ -24,12 +26,13 @@ from typing import (
 import anyio
 from mcp.shared.exceptions import McpError
 from mcp.types import ErrorData, Icon, ToolAnnotations
-from pydantic import Field
+from pydantic import Field, TypeAdapter
+from pydantic import ValidationError as PydanticValidationError
 from pydantic.json_schema import SkipJsonSchema
 
 import fastmcp
 from fastmcp.decorators import get_fastmcp_meta, resolve_task_config
-from fastmcp.exceptions import FastMCPDeprecationWarning
+from fastmcp.exceptions import FastMCPDeprecationWarning, ValidationError
 from fastmcp.tools.base import (
     Tool,
     ToolResult,
@@ -55,6 +58,75 @@ if TYPE_CHECKING:
     from docket import Docket
     from docket.execution import Execution
 
+
+class _ToolBodyError(Exception):
+    """Marks a ``pydantic.ValidationError`` raised while executing a tool's body.
+
+    Pydantic validates a tool's arguments *before* invoking the body, so a bare
+    ``pydantic.ValidationError`` surfacing from the call adapter is unambiguously
+    an argument-validation failure (a bad call). Errors a tool raises from its
+    own body — e.g. constructing a model from upstream data — are a different
+    class of problem (a server-side bug) that must not be reclassified as a bad
+    call. We wrap the body so those are tagged and can be told apart. See #4128.
+    """
+
+
+@lru_cache(maxsize=5000)
+def _wrap_body_errors(fn: Callable[..., Any]) -> Callable[..., Any]:
+    """Wrap ``fn`` so a ``pydantic.ValidationError`` raised by its body is
+    re-raised as ``_ToolBodyError``.
+
+    The wrapper preserves ``fn``'s signature and annotations so the cached
+    ``TypeAdapter`` validates arguments identically — only body execution is
+    affected. Argument validation happens before the wrapper is called, so it
+    keeps raising a bare ``pydantic.ValidationError``.
+    """
+    if is_coroutine_function(fn):
+
+        async def wrapper(*args: Any, **kwargs: Any) -> Any:
+            try:
+                return await fn(*args, **kwargs)
+            except PydanticValidationError as e:
+                raise _ToolBodyError from e
+    else:
+
+        def wrapper(*args: Any, **kwargs: Any) -> Any:
+            try:
+                return fn(*args, **kwargs)
+            except PydanticValidationError as e:
+                raise _ToolBodyError from e
+
+    # Mirror the original callable so TypeAdapter builds the identical schema and
+    # binds arguments the same way. Annotations must cover every signature
+    # parameter or pydantic's call-schema generation raises KeyError — so prefer
+    # resolved type hints (handles string/forward refs) but fall back to the
+    # signature's own annotations, which is the only source for callables like
+    # functools.partial that carry no __annotations__.
+    try:
+        resolved_hints = get_type_hints(fn, include_extras=True)
+    except Exception:
+        resolved_hints = {}
+    sig = inspect.signature(fn)
+    annotations: dict[str, Any] = {}
+    for param_name, param in sig.parameters.items():
+        if param_name in resolved_hints:
+            annotations[param_name] = resolved_hints[param_name]
+        elif param.annotation is not inspect.Parameter.empty:
+            annotations[param_name] = param.annotation
+    if "return" in resolved_hints:
+        annotations["return"] = resolved_hints["return"]
+    elif sig.return_annotation is not inspect.Signature.empty:
+        annotations["return"] = sig.return_annotation
+
+    wrapper.__signature__ = sig  # type: ignore[attr-defined]  # ty: ignore[invalid-assignment]
+    wrapper.__annotations__ = annotations
+    wrapper.__name__ = getattr(fn, "__name__", "wrapper")
+    wrapper.__doc__ = getattr(fn, "__doc__", None)
+    wrapper.__module__ = getattr(fn, "__module__", wrapper.__module__)
+    wrapper.__qualname__ = getattr(fn, "__qualname__", wrapper.__qualname__)
+    return wrapper
+
+
 F = TypeVar("F", bound=Callable[..., Any])
 
 
@@ -317,59 +389,88 @@ class FunctionTool(Tool):
         wrapper_fn = without_injected_parameters(
             self.fn, run_in_thread=self.run_in_thread
         )
-        type_adapter = get_cached_typeadapter(wrapper_fn)
+        # Tag pydantic errors raised by the body so they can be distinguished
+        # from argument-validation errors (which pydantic raises first). See #4128.
+        exec_fn = _wrap_body_errors(wrapper_fn)
+        type_adapter = get_cached_typeadapter(exec_fn)
+        exec_is_async = is_coroutine_function(wrapper_fn)
 
-        # Apply timeout if configured. Combining timeout with
-        # run_in_thread=False on a sync function is rejected at
-        # registration (see FunctionTool.from_function), so the timeout
-        # path here only needs to handle async and threadpool-sync.
-        if self.timeout is not None:
-            try:
-                with anyio.fail_after(self.timeout):
-                    # Thread pool execution for sync functions, direct await for async
-                    if is_coroutine_function(wrapper_fn):
-                        result = await type_adapter.validate_python(arguments)
-                    else:
-                        # Sync function: run in threadpool to avoid blocking
-                        result = await call_sync_fn_in_threadpool(
-                            type_adapter.validate_python, arguments
+        try:
+            if self.timeout is not None:
+                try:
+                    with anyio.fail_after(self.timeout):
+                        result = await self._execute(
+                            type_adapter, exec_is_async, arguments
                         )
-                        # Handle sync wrappers that return awaitables
-                        if inspect.isawaitable(result):
-                            result = await result
-                    # Materialize generators inside timeout scope so slow
-                    # generators don't run past the configured timeout
-                    result = await self._materialize_generator(result)
-            except TimeoutError:
-                logger.warning(
-                    f"Tool '{self.name}' timed out after {self.timeout}s. "
-                    f"Consider using task=True for long-running operations. "
-                    f"See https://gofastmcp.com/servers/tasks"
-                )
-                raise McpError(
-                    ErrorData(
-                        code=-32000,
-                        message=f"Tool '{self.name}' execution timed out after {self.timeout}s",
+                except TimeoutError:
+                    logger.warning(
+                        f"Tool '{self.name}' timed out after {self.timeout}s. "
+                        f"Consider using task=True for long-running operations. "
+                        f"See https://gofastmcp.com/servers/tasks"
                     )
-                ) from None
-        else:
-            # No timeout: use existing execution path
-            if is_coroutine_function(wrapper_fn):
-                result = await type_adapter.validate_python(arguments)
-            elif self.run_in_thread:
-                result = await call_sync_fn_in_threadpool(
-                    type_adapter.validate_python, arguments
-                )
-                if inspect.isawaitable(result):
-                    result = await result
+                    raise McpError(
+                        ErrorData(
+                            code=-32000,
+                            message=f"Tool '{self.name}' execution timed out after {self.timeout}s",
+                        )
+                    ) from None
             else:
-                result = type_adapter.validate_python(arguments)
-                if inspect.isawaitable(result):
-                    result = await result
-            result = await self._materialize_generator(result)
+                result = await self._execute(type_adapter, exec_is_async, arguments)
+        except PydanticValidationError as e:
+            # Body errors are re-raised as _ToolBodyError, so a bare pydantic
+            # ValidationError here is an argument-validation failure (a bad call).
+            # Convert it to fastmcp's ValidationError so the middleware chain and
+            # downstream error taxonomy (e.g. Sentry filters) can treat it as a
+            # client error rather than a server bug.
+            raise ValidationError(str(e), log_level=logging.WARNING) from e
+        except _ToolBodyError as e:
+            # The tool's own body raised a pydantic ValidationError. Surface the
+            # original so it is treated as a server-side error, hiding the
+            # internal sentinel while preserving the error's own chained cause.
+            original = e.__cause__
+            assert original is not None
+            raise original from original.__cause__
 
         return self.convert_result(result)
 
+    async def _execute(
+        self,
+        type_adapter: TypeAdapter[Any],
+        exec_is_async: bool,
+        arguments: dict[str, Any],
+    ) -> Any:
+        """Validate arguments and execute the tool body.
+
+        Argument validation runs first and raises a bare
+        ``pydantic.ValidationError`` on bad input. Body execution (awaiting the
+        result and materializing generators) is wrapped so any pydantic error it
+        raises is tagged as ``_ToolBodyError``.
+        """
+        # Combining timeout with run_in_thread=False on a sync function is
+        # rejected at registration (see FunctionTool.from_function), so this only
+        # needs to handle async and threadpool-sync under a timeout.
+        if exec_is_async:
+            # Argument validation is synchronous; the body runs on await below.
+            result = type_adapter.validate_python(arguments)
+        elif self.run_in_thread:
+            # Sync function: run in threadpool to avoid blocking the event loop.
+            result = await call_sync_fn_in_threadpool(
+                type_adapter.validate_python, arguments
+            )
+        else:
+            result = type_adapter.validate_python(arguments)
+
+        try:
+            if inspect.isawaitable(result):
+                result = await result
+            # Materialize generators (here, so slow generators are still bound by
+            # any configured timeout scope).
+            return await self._materialize_generator(result)
+        except PydanticValidationError as e:
+            # A pydantic error from awaiting the result or materializing a
+            # generator is body execution, not argument validation.
+            raise _ToolBodyError from e
+
     @staticmethod
     async def _materialize_generator(result: Any) -> Any:
         """Consume generators/async generators into lists.
@@ -450,7 +551,13 @@ class FunctionTool(Tool):
             if annotation is None:
                 continue
             adapter = get_cached_typeadapter(annotation)
-            coerced[name] = adapter.validate_python(value)
+            try:
+                coerced[name] = adapter.validate_python(value)
+            except PydanticValidationError as e:
+                # Argument coercion failure on the task path is a bad call, just
+                # like the synchronous path — surface it as fastmcp's
+                # ValidationError so it is classified consistently (see #4128).
+                raise ValidationError(str(e), log_level=logging.WARNING) from e
         return coerced
 
 

tests/server/providers/local_provider_tools/test_parameters.py

@@ -109,7 +109,7 @@ class TestToolParameters:
         assert result.content[0].data == base64.b64encode(b"fake png data").decode()
 
     async def test_tool_with_invalid_input(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -149,7 +149,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": True}
 
     async def test_annotated_field_validation(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -164,7 +164,7 @@ class TestToolParameters:
             await mcp.call_tool("analyze", {"x": 0})
 
     async def test_default_field_validation(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -179,7 +179,7 @@ class TestToolParameters:
             await mcp.call_tool("analyze", {"x": 0})
 
     async def test_default_field_is_still_required_if_no_default_specified(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -191,7 +191,7 @@ class TestToolParameters:
             await mcp.call_tool("analyze", {})
 
     async def test_literal_type_validation_error(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -216,7 +216,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": "a"}
 
     async def test_enum_type_validation_error(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -251,7 +251,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": "red"}
 
     async def test_union_type_validation(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -285,7 +285,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": str(test_path)}
 
     async def test_path_type_error(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -310,7 +310,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": str(test_uuid)}
 
     async def test_uuid_type_error(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 
@@ -344,7 +344,7 @@ class TestToolParameters:
         assert result.structured_content == {"result": "2021-01-01T00:00:00"}
 
     async def test_datetime_type_error(self):
-        from pydantic import ValidationError
+        from fastmcp.exceptions import ValidationError
 
         mcp = FastMCP()
 

tests/server/test_dependencies.py

@@ -450,10 +450,11 @@ async def test_argument_validation_with_dependencies(mcp: FastMCP):
     assert result.structured_content is not None
     assert result.structured_content["result"] == "age=25"
 
-    # Invalid argument type should fail validation
-    import pydantic
+    # Invalid argument type should fail validation. Argument-validation errors
+    # surface as fastmcp's ValidationError (see #4128), not raw pydantic.
+    from fastmcp.exceptions import ValidationError
 
-    with pytest.raises(pydantic.ValidationError):
+    with pytest.raises(ValidationError):
         await mcp.call_tool("validated_tool", {"age": "not a number"})
 
 
@@ -598,10 +599,12 @@ async def test_external_user_cannot_override_dependency(mcp: FastMCP):
     assert result.structured_content is not None
     assert "admin=not_admin" in result.structured_content["result"]
 
-    # Try to override dependency - rejected (not in schema)
-    import pydantic
+    # Try to override dependency - rejected (not in schema). The rejection is an
+    # argument-validation error, so it surfaces as fastmcp's ValidationError
+    # (see #4128).
+    from fastmcp.exceptions import ValidationError
 
-    with pytest.raises(pydantic.ValidationError):
+    with pytest.raises(ValidationError):
         await mcp.call_tool("check_permission", {"action": "read", "admin": "hacker"})
 
 

tests/tools/tool/test_argument_validation.py

@@ -0,0 +1,125 @@
+"""Tests for distinguishing argument-validation errors from tool-body errors.
+
+See https://github.com/PrefectHQ/fastmcp/issues/4128: a bad call (invalid
+arguments) should surface as fastmcp's ``ValidationError`` so downstream error
+taxonomy (middleware, Sentry filters) can treat it as a client error, while a
+``pydantic.ValidationError`` raised by the tool's own body is a server-side bug
+and must propagate unchanged.
+"""
+
+from typing import Annotated, Any
+
+import pytest
+from pydantic import BaseModel, Field
+from pydantic import ValidationError as PydanticValidationError
+
+from fastmcp.exceptions import ValidationError
+from fastmcp.tools.base import Tool
+
+
+class _Inner(BaseModel):
+    x: int
+
+
+class TestArgumentValidationErrors:
+    """Invalid arguments are converted to fastmcp's ValidationError."""
+
+    async def test_async_constraint_violation(self):
+        async def tool_fn(n: Annotated[int, Field(le=10)]) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            await tool.run({"n": 20})
+
+    async def test_sync_constraint_violation(self):
+        def tool_fn(n: Annotated[int, Field(le=10)]) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            await tool.run({"n": 20})
+
+    async def test_wrong_type_is_argument_error(self):
+        async def tool_fn(n: int) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            await tool.run({"n": "not-an-int"})
+
+    async def test_missing_required_argument(self):
+        async def tool_fn(n: int) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            await tool.run({})
+
+
+class TestToolBodyErrors:
+    """A pydantic error raised by the body is NOT reclassified as a bad call."""
+
+    async def test_async_body_pydantic_error_propagates(self):
+        async def tool_fn(data: str) -> int:
+            bad_value: Any = "not-an-int"
+            _Inner(x=bad_value)  # raises a pydantic ValidationError from the body
+            return 1
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(PydanticValidationError):
+            await tool.run({"data": "valid"})
+        # And it must not be fastmcp's ValidationError.
+        with pytest.raises(PydanticValidationError) as exc_info:
+            await tool.run({"data": "valid"})
+        assert not isinstance(exc_info.value, ValidationError)
+
+    async def test_sync_body_pydantic_error_propagates(self):
+        def tool_fn(data: str) -> int:
+            bad_value: Any = "not-an-int"
+            _Inner(x=bad_value)  # raises a pydantic ValidationError from the body
+            return 1
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(PydanticValidationError):
+            await tool.run({"data": "valid"})
+
+
+class TestTaskArgumentValidation:
+    """The task-execution path (coerce_task_arguments) converts arg errors too."""
+
+    def test_coerce_task_arguments_wrong_type(self):
+        def tool_fn(n: int) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            tool.coerce_task_arguments({"n": "not-an-int"})
+
+    def test_coerce_task_arguments_constraint_violation(self):
+        def tool_fn(n: Annotated[int, Field(le=10)]) -> int:
+            return n
+
+        tool = Tool.from_function(tool_fn)
+        with pytest.raises(ValidationError):
+            tool.coerce_task_arguments({"n": 20})
+
+
+class TestValidCallsStillWork:
+    """Regression: the happy path is unaffected."""
+
+    async def test_async_valid_call(self):
+        async def tool_fn(n: Annotated[int, Field(le=10)]) -> int:
+            return n * 2
+
+        tool = Tool.from_function(tool_fn)
+        result = await tool.run({"n": 5})
+        assert result.structured_content == {"result": 10}
+
+    async def test_sync_valid_call(self):
+        def tool_fn(n: int) -> int:
+            return n + 1
+
+        tool = Tool.from_function(tool_fn)
+        result = await tool.run({"n": 5})
+        assert result.structured_content == {"result": 6}