feat(mcp): validate and wrap async functions as code tools

redisvl/mcp/config.py

@@ -57,6 +57,32 @@ def builtin_tool_names() -> frozenset[str]:
     return _BUILTIN_TOOL_NAMES
 
 
+def validate_tool_name(name: str, *, kind: str) -> None:
+    """Raise ``ValueError`` unless ``name`` is usable for a custom tool.
+
+    Shared by every kind of custom tool so that one set of rules decides what a
+    tool may be called. ``kind`` names the source in the message, so an error
+    points at the config section or decorator that produced the name.
+    """
+    # fullmatch, because `$` also matches before a trailing newline.
+    if not _TOOL_NAME_PATTERN.fullmatch(name):
+        raise ValueError(
+            f"{kind} name '{name}' is invalid; names must start with a "
+            "lowercase letter and contain only lowercase letters, digits, "
+            "hyphens, or underscores (max 64 characters)"
+        )
+    if name in builtin_tool_names():
+        raise ValueError(
+            f"{kind} name '{name}' collides with a built-in tool; "
+            "built-in names are reserved"
+        )
+    if name.startswith(_RESERVED_TOOL_NAME_PREFIXES):
+        raise ValueError(
+            f"{kind} name '{name}' uses a reserved prefix "
+            f"({', '.join(_RESERVED_TOOL_NAME_PREFIXES)})"
+        )
+
+
 class MCPRuntimeConfig(BaseModel):
     """Runtime limits and validated field mappings for MCP requests."""
 
@@ -765,22 +791,7 @@ def _validate_profile(self) -> "MCPCustomToolConfig":
             raise ValueError(
                 f"custom_tools '{self.name}' description must not be blank"
             )
-        if not _TOOL_NAME_PATTERN.match(self.name):
-            raise ValueError(
-                f"custom_tools name '{self.name}' is invalid; names must start with a "
-                "lowercase letter and contain only lowercase letters, digits, "
-                "hyphens, or underscores (max 64 characters)"
-            )
-        if self.name in builtin_tool_names():
-            raise ValueError(
-                f"custom_tools name '{self.name}' collides with a built-in tool; "
-                "built-in names are reserved"
-            )
-        if self.name.startswith(_RESERVED_TOOL_NAME_PREFIXES):
-            raise ValueError(
-                f"custom_tools name '{self.name}' uses a reserved prefix "
-                f"({', '.join(_RESERVED_TOOL_NAME_PREFIXES)})"
-            )
+        validate_tool_name(self.name, kind="custom_tools")
 
         unknown_params = sorted(set(self.params) - _PROFILE_PARAM_NAMES)
         if unknown_params:

redisvl/mcp/server.py

@@ -266,7 +266,15 @@ async def run_guarded(
             self._close_awaitable(awaitable)
             raise RuntimeError("MCP server is not running")
 
-        async with semaphore:
+        try:
+            await semaphore.acquire()
+        except BaseException:
+            # Cancelled while waiting for a slot, as by a code tool's whole-call
+            # timeout, so the awaitable never started.
+            self._close_awaitable(awaitable)
+            raise
+
+        try:
             async with self._request_state_lock:
                 if self._lifecycle_state is not _LifecycleState.RUNNING:
                     self._close_awaitable(awaitable)
@@ -286,6 +294,8 @@ async def run_guarded(
                     self._active_requests -= 1
                     if self._active_requests == 0:
                         self._active_requests_drained.set()
+        finally:
+            semaphore.release()
 
     @staticmethod
     def _build_vectorizer(binding: MCPIndexBindingConfig) -> Any:

redisvl/mcp/tools/code.py

@@ -0,0 +1,230 @@
+"""Code tools: async Python functions registered as MCP tools.
+
+A code tool is an async function whose first parameter is a
+:class:`~redisvl.mcp.context.ToolContext` and whose remaining, annotated
+parameters are the arguments the model supplies. Its docstring (or an explicit
+description) is what the model reads when choosing a tool.
+
+Everything that makes a built-in safe is applied around the function rather
+than left to its author: the read or write scope and any per-tool scope are
+checked before it runs, the context it receives can only reach Redis through a
+guarded, allow-listed handle, the whole call is bounded by the binding's
+request timeout, and any exception it raises is mapped to the stable MCP error
+contract instead of leaking a traceback.
+"""
+
+import asyncio
+import inspect
+import typing
+from collections.abc import Callable
+from dataclasses import dataclass
+from typing import Any
+
+from pydantic import TypeAdapter
+
+from redisvl.mcp.auth import ensure_read_scope, ensure_tool_scope, ensure_write_scope
+from redisvl.mcp.config import validate_tool_name
+from redisvl.mcp.context import ToolContext
+from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError, map_exception
+
+_ACCEPTED_KINDS = (
+    inspect.Parameter.POSITIONAL_OR_KEYWORD,
+    inspect.Parameter.KEYWORD_ONLY,
+)
+
+
+@dataclass(frozen=True)
+class CodeToolSpec:
+    """One validated code tool, ready to register against a binding."""
+
+    fn: Callable[..., Any]
+    name: str
+    description: str
+    index: str | None
+    writes: bool
+    scope: str | None
+    signature: inspect.Signature
+    annotations: dict[str, Any]
+
+
+def _qualified(fn: Callable[..., Any]) -> str:
+    return f"{getattr(fn, '__module__', '?')}.{getattr(fn, '__qualname__', fn)}"
+
+
+def _tool_signature(
+    fn: Callable[..., Any],
+) -> tuple[inspect.Signature, dict[str, Any]]:
+    """Return the model-facing signature: the function's, minus the context."""
+    where = _qualified(fn)
+    try:
+        # Resolved rather than read raw, so a module using postponed
+        # annotations is checked against the real types.
+        hints = typing.get_type_hints(fn, include_extras=True)
+    except Exception as exc:
+        raise TypeError(
+            f"code tool {where} has annotations that cannot be resolved: {exc}"
+        ) from exc
+
+    parameters = list(inspect.signature(fn).parameters.values())
+    # Positional, because the wrapper passes the context positionally; a
+    # keyword-only one would be accepted here and fail on every call.
+    if (
+        not parameters
+        or hints.get(parameters[0].name) is not ToolContext
+        or parameters[0].kind
+        not in (
+            inspect.Parameter.POSITIONAL_ONLY,
+            inspect.Parameter.POSITIONAL_OR_KEYWORD,
+        )
+    ):
+        raise TypeError(
+            f"code tool {where} must take a ToolContext as its first, positional "
+            "parameter"
+        )
+
+    exposed = []
+    annotations: dict[str, Any] = {}
+    for parameter in parameters[1:]:
+        if parameter.kind not in _ACCEPTED_KINDS:
+            raise TypeError(
+                f"code tool {where} parameter '{parameter.name}' must be a plain "
+                "named parameter; *args, **kwargs and positional-only "
+                "parameters cannot be offered to a model"
+            )
+        if parameter.name not in hints:
+            raise TypeError(
+                f"code tool {where} parameter '{parameter.name}' needs a type "
+                "annotation; it is what the tool's input schema is built from"
+            )
+        annotation = hints[parameter.name]
+        try:
+            # Built here so a type the input schema cannot express fails at
+            # declaration, not at startup after every binding has connected.
+            TypeAdapter(annotation).json_schema()
+        except Exception as exc:
+            raise TypeError(
+                f"code tool {where} parameter '{parameter.name}' has a type "
+                f"that no input schema can describe: {exc}"
+            ) from exc
+        # Keyword-only, because the wrapper is always called with keyword
+        # arguments decoded from the model's JSON.
+        exposed.append(
+            parameter.replace(
+                kind=inspect.Parameter.KEYWORD_ONLY, annotation=annotation
+            )
+        )
+        annotations[parameter.name] = annotation
+
+    return_annotation = hints.get("return", inspect.Signature.empty)
+    if return_annotation is not inspect.Signature.empty:
+        annotations["return"] = return_annotation
+    return (
+        inspect.Signature(exposed, return_annotation=return_annotation),
+        annotations,
+    )
+
+
+def build_code_tool_spec(
+    fn: Callable[..., Any],
+    *,
+    name: str | None = None,
+    namespace: str | None = None,
+    index: str | None = None,
+    writes: bool = False,
+    scope: str | None = None,
+    description: str | None = None,
+) -> CodeToolSpec:
+    """Validate ``fn`` as a code tool and return its spec.
+
+    Everything checkable without a running server is checked here, so a
+    malformed tool fails where it is declared rather than at startup.
+    Whether ``index`` names a real binding, and whether a writing tool is
+    allowed on it, depends on the server's config and is checked at startup.
+    """
+    where = _qualified(fn)
+    if not inspect.iscoroutinefunction(fn):
+        raise TypeError(f"code tool {where} must be an async function")
+
+    signature, annotations = _tool_signature(fn)
+
+    resolved_description = (
+        description if description is not None else inspect.getdoc(fn)
+    )
+    if not resolved_description or not resolved_description.strip():
+        raise ValueError(
+            f"code tool {where} needs a description: give it a docstring or pass "
+            "description=; it is what the model chooses tools by"
+        )
+
+    resolved_name = name if name is not None else fn.__name__.replace("_", "-")
+    if namespace is not None:
+        resolved_name = f"{namespace}-{resolved_name}"
+    validate_tool_name(resolved_name, kind=f"code tool {where}")
+
+    if scope is not None and not scope.strip():
+        raise ValueError(f"code tool {where} scope must not be blank")
+    if scope is not None and scope != scope.strip():
+        # Scopes are matched exactly, so a padded one would refuse every caller.
+        raise ValueError(
+            f"code tool {where} scope {scope!r} has surrounding whitespace"
+        )
+
+    return CodeToolSpec(
+        fn=fn,
+        name=resolved_name,
+        description=resolved_description.strip(),
+        index=index,
+        writes=writes,
+        scope=scope,
+        signature=signature,
+        annotations=annotations,
+    )
+
+
+def register_code_tool(server: Any, spec: CodeToolSpec, binding_id: str) -> None:
+    """Register one code tool against ``binding_id``."""
+
+    async def code_tool(**kwargs: Any) -> Any:
+        # Before anything else, so a caller without the scope never reaches the
+        # binding or the function. The tool's own scope is required on top of
+        # the server-wide one, so it can only make a tool stricter. A writing
+        # tool needs the read scope too, because its handle can also read.
+        ensure_read_scope(server)
+        if spec.writes:
+            ensure_write_scope(server)
+        ensure_tool_scope(server, spec.scope)
+
+        try:
+            runtime = server.resolve_binding(binding_id)
+            if spec.writes and runtime.effective_read_only:
+                # Startup already refuses this pairing; checked again per call,
+                # as upsert_records does, so a write never depends on that
+                # check alone.
+                raise RedisVLMCPError(
+                    f"index '{binding_id}' is read-only",
+                    code=MCPErrorCode.FORBIDDEN,
+                    retryable=False,
+                )
+            ctx = ToolContext.for_request(server, runtime, writes=spec.writes)
+            timeout = runtime.binding.runtime.request_timeout_seconds
+            try:
+                # The handle bounds each Redis call; this bounds the tool's own
+                # code too, so a tool cannot hold a request open indefinitely.
+                return await asyncio.wait_for(spec.fn(ctx, **kwargs), timeout)
+            except asyncio.TimeoutError as exc:
+                raise RedisVLMCPError(
+                    f"tool '{spec.name}' did not finish within {timeout:g}s",
+                    code=MCPErrorCode.BACKEND_UNAVAILABLE,
+                    retryable=True,
+                ) from exc
+        except RedisVLMCPError:
+            raise
+        except Exception as exc:
+            raise map_exception(exc) from exc
+
+    code_tool.__name__ = spec.name.replace("-", "_")
+    code_tool.__doc__ = spec.description
+    code_tool.__signature__ = spec.signature  # type: ignore[attr-defined]
+    code_tool.__annotations__ = dict(spec.annotations)
+
+    server.tool(name=spec.name, description=spec.description)(code_tool)