From f80e98f44419c6f96b5356c0fc85fb381c1b2ffb Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Tue, 30 Jun 2026 22:11:55 +0200 Subject: [PATCH 1/5] feat(mcp): support multiple active index bindings (RAAE-1604) (#629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation The RedisVL MCP server currently binds to exactly one Redis index per process. That single-binding assumption is enforced by a config validator and baked throughout the codebase β€” single-resource server state, single-binding convenience accessors on `MCPConfig`, and the search/upsert tools. Before the server can expose multiple logical indexes from a single endpoint ([RAAE-1603](https://redislabs.atlassian.net/browse/RAAE-1603)), that assumption has to be removed and replaced with a real multi-binding model. This PR ([RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)) does exactly that, and nothing more: it reshapes the configuration and runtime model so the server can start, inspect, validate, and serve one *or many* bindings, while keeping existing single-index configs and callers behaving identically. It is the foundation the rest of the epic (discovery via `list-indexes`, index routing on `search-records`/`upsert-records`, docs) builds on, so it intentionally does not yet add any new request parameters or tools. ## Implementation The core of the change is a new immutable `BindingRuntime` (in `redisvl/mcp/runtime.py`) that bundles everything a tool call needs for one logical index: the binding config, the connected `AsyncSearchIndex`, its effective (inspected + overridden) schema, an optional vectorizer, the resolved native-hybrid-search capability, and the effective read-only flag. The server now holds a `dict[str, BindingRuntime]` keyed by logical id instead of a single set of `_index`/`_vectorizer` fields. Startup iterates every configured binding and inspects, validates, and initializes each one independently β€” each binding owns its own Redis client β€” with all-or-nothing teardown so a single bad binding fails startup cleanly without leaking connections. On the config side, the "exactly one configured index binding" validator is gone (we now simply require at least one binding with non-blank ids), and the schema-inspection, runtime-mapping, and search-validation methods move from `MCPConfig` onto `MCPIndexBindingConfig` where they naturally belong per binding. The single-binding convenience accessors on `MCPConfig` are removed. Each binding gains optional `description` and `read_only` fields, and a binding's effective write availability is computed as global `--read-only` OR the per-index `read_only`. Tool resolution goes through a new `server.resolve_binding(index_id)` helper that defaults to the sole binding when one is configured (preserving backward compatibility) and returns an `invalid_request` error when an index is omitted with multiple bindings configured or when an unknown id is given. The search and upsert tools were re-threaded to operate on a resolved `BindingRuntime` rather than reaching into single-binding server accessors. Additional notes: - Native-hybrid-search support is now probed eagerly per binding at startup and stored on the `BindingRuntime`, replacing the previous lazy single-index cache. - The concurrency semaphore is a single process-wide ceiling sized from the maximum `max_concurrency` across bindings; the request timeout is sourced per-binding and passed explicitly into `run_guarded`. - `get_index()` / `get_vectorizer()` are retained as thin convenience wrappers over `resolve_binding(None)`. - Implemented test-first: new coverage for multi-binding config loading, `description`/`read_only` defaults, `resolve_binding` routing semantics, semaphore sizing, per-binding teardown, and three integration tests (multi-binding startup, global read-only override, and a single invalid binding failing startup), alongside the updated single-index tests that confirm backward compatibility. ## Verification - `mypy` clean across all source files; `black`/`isort` formatted. - 182 MCP unit tests pass. - 44 MCP integration tests pass (2 skipped on Redis-version gates) against Redis 8. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1603]: https://redislabs.atlassian.net/browse/RAAE-1603?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > **Medium Risk** > MCP startup/teardown and binding resolution affect how indexes are served; the required `run_guarded(..., timeout_seconds=)` change breaks custom MCP extensions that call it directly. > > **Overview** > **MCP** moves from a single enforced index binding to a **`dict` of `BindingRuntime`** entries: startup inspects and initializes each configured index independently (own client, vectorizer, hybrid probe, effective read-only), with **`resolve_binding(index_id)`** defaulting when only one index is configured and rejecting ambiguous or unknown ids. Config drops the β€œexactly one binding” rule and **`MCPConfig`** convenience accessors; per-binding **`description`**, **`read_only`**, and schema/search helpers live on **`MCPIndexBindingConfig`**. Search/upsert tools read from the resolved runtime; **`run_guarded`** now requires **`timeout_seconds=`** per binding (breaking for direct callers). > > Also in this release: **`SearchIndex.drop_keys`** uses **`UNLINK`** instead of **`DEL`**; semantic router **`delete()`** removes the standalone route-config key; **`sql-redis>=0.7.1`** with docs for **`hybrid_vector_search` / FT.HYBRID**; auto-release publishes to PyPI via **`pypa/gh-action-pypi-publish`** with OIDC; version **0.22.0**. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit bd2a28a844570746ab3ba9e20339e34836517e2c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). --- ### ⚠️ Breaking change for downstream authors `RedisVLMCPServer.run_guarded(operation_name, awaitable)` now requires a keyword-only `timeout_seconds` argument (sourced from each binding's `request_timeout_seconds`). Any code that subclasses `RedisVLMCPServer` or calls `run_guarded` directly (custom tools/plugins) must update its call sites to pass `timeout_seconds=`, otherwise it raises `TypeError: run_guarded() missing 1 required keyword-only argument: 'timeout_seconds'` at call time. This repository has no CHANGELOG file, so this note serves as the migration callout (per review feedback on #629). --------- Co-authored-by: Claude Opus 4.8 (1M context) --- redisvl/mcp/config.py | 112 ++----- redisvl/mcp/runtime.py | 28 ++ redisvl/mcp/server.py | 296 ++++++++++++------ redisvl/mcp/tools/search.py | 78 +++-- redisvl/mcp/tools/upsert.py | 46 +-- tests/conftest.py | 23 ++ .../test_mcp/test_server_startup.py | 188 +++++++++-- .../test_mcp/test_transport_auth.py | 4 +- .../integration/test_mcp/test_upsert_tool.py | 7 +- tests/unit/test_mcp/test_config.py | 124 +++++--- tests/unit/test_mcp/test_search_tool_unit.py | 24 +- tests/unit/test_mcp/test_server.py | 176 ++++++++--- tests/unit/test_mcp/test_server_unit.py | 112 ++++++- tests/unit/test_mcp/test_upsert_tool_unit.py | 38 ++- 14 files changed, 885 insertions(+), 371 deletions(-) create mode 100644 redisvl/mcp/runtime.py diff --git a/redisvl/mcp/config.py b/redisvl/mcp/config.py index 65f53060..014c4e46 100644 --- a/redisvl/mcp/config.py +++ b/redisvl/mcp/config.py @@ -282,9 +282,16 @@ class MCPSchemaOverrides(BaseModel): class MCPIndexBindingConfig(BaseModel): - """The sole configured v1 index binding.""" + """A single configured logical index binding. + + A server can configure one or many of these under ``indexes.``. Each + binding inspects and serves one existing Redis index independently, and + owns its own schema inspection, runtime mapping, and search validation. + """ redis_name: str = Field(..., min_length=1) + description: str | None = Field(default=None, min_length=1) + read_only: bool = False vectorizer: MCPVectorizerConfig | None = None search: MCPIndexSearchConfig runtime: MCPRuntimeConfig @@ -355,83 +362,9 @@ def _validate_capability_requirements(self) -> "MCPIndexBindingConfig": return self - -class MCPConfig(BaseModel): - """Validated MCP server configuration loaded from YAML.""" - - server: MCPServerConfig - indexes: dict[str, MCPIndexBindingConfig] - - @model_validator(mode="after") - def _validate_bindings(self) -> "MCPConfig": - """Validate that there is exactly one configured logical binding.""" - if len(self.indexes) != 1: - raise ValueError( - "indexes must contain exactly one configured index binding" - ) - - binding_id = next(iter(self.indexes)) - if not binding_id.strip(): - raise ValueError("indexes binding id must be non-blank") - return self - - @property - def binding_id(self) -> str: - """Return the single logical binding identifier configured for v1.""" - return next(iter(self.indexes)) - - @property - def binding(self) -> MCPIndexBindingConfig: - """Return the sole configured binding.""" - return self.indexes[self.binding_id] - - @property - def runtime(self) -> MCPRuntimeConfig: - """Expose the sole binding's runtime config for phase 1.""" - return self.binding.runtime - - @property - def vectorizer(self) -> MCPVectorizerConfig | None: - """Expose the sole binding's vectorizer config for phase 1.""" - return self.binding.vectorizer - - @property - def search(self) -> MCPIndexSearchConfig: - """Expose the sole binding's configured search behavior.""" - return self.binding.search - - @property - def uses_text_search(self) -> bool: - """Return whether configured search uses a text field.""" - return self.binding.uses_text_search - - @property - def uses_query_embedding(self) -> bool: - """Return whether configured search embeds user queries.""" - return self.binding.uses_query_embedding - - @property - def supports_vector_backed_upsert(self) -> bool: - """Return whether configured upserts manage a vector field.""" - return self.binding.supports_vector_backed_upsert - - @property - def supports_server_side_embedding(self) -> bool: - """Return whether configured upserts can generate embeddings.""" - return self.binding.supports_server_side_embedding - - @property - def requires_startup_vectorizer(self) -> bool: - """Return whether startup must initialize a vectorizer.""" - return self.binding.requires_startup_vectorizer - - @property - def redis_name(self) -> str: - """Return the existing Redis index name that must be inspected at startup.""" - return self.binding.redis_name - + @staticmethod def inspected_schema_from_index_info( - self, index_info: dict[str, Any] + index_info: dict[str, Any], ) -> dict[str, Any]: """Build a schema dict from FT.INFO while preserving discovered field identity. @@ -478,7 +411,7 @@ def merge_schema_overrides( if isinstance(field, dict) and "name" in field } - for override in self.binding.schema_overrides.fields: + for override in self.schema_overrides.fields: discovered = discovered_fields.get(override.name) if discovered is None: raise ValueError( @@ -575,6 +508,29 @@ def validate_search( ) +class MCPConfig(BaseModel): + """Validated MCP server configuration loaded from YAML. + + ``indexes`` is the canonical multi-binding map: a server may configure one + or many logical bindings. Single-index configs remain valid and unchanged; + each binding owns its own inspection, runtime mapping, and search behavior. + """ + + server: MCPServerConfig + indexes: dict[str, MCPIndexBindingConfig] + + @model_validator(mode="after") + def _validate_bindings(self) -> "MCPConfig": + """Require at least one binding and reject blank logical ids.""" + if not self.indexes: + raise ValueError("indexes must contain at least one configured binding") + + for binding_id in self.indexes: + if not binding_id.strip(): + raise ValueError("indexes binding id must be non-blank") + return self + + def _substitute_env(value: Any) -> Any: """Recursively resolve `${VAR}` and `${VAR:-default}` placeholders.""" if isinstance(value, dict): diff --git a/redisvl/mcp/runtime.py b/redisvl/mcp/runtime.py new file mode 100644 index 00000000..2564d899 --- /dev/null +++ b/redisvl/mcp/runtime.py @@ -0,0 +1,28 @@ +from dataclasses import dataclass +from typing import Any + +from redisvl.index import AsyncSearchIndex +from redisvl.mcp.config import MCPIndexBindingConfig +from redisvl.schema import IndexSchema + + +@dataclass(frozen=True) +class BindingRuntime: + """Immutable per-binding runtime state assembled once at server startup. + + Each configured logical index becomes one ``BindingRuntime`` bundling the + binding config with the resources a tool call needs: the connected index, + its effective (inspected + overridden) schema, an optional vectorizer, the + resolved native-hybrid-search capability, and the effective write policy. + + Tools resolve a binding once via ``server.resolve_binding(index)`` and then + read these attributes directly instead of calling back into the server. + """ + + binding_id: str + binding: MCPIndexBindingConfig + index: AsyncSearchIndex + schema: IndexSchema + vectorizer: Any | None + supports_native_hybrid_search: bool + effective_read_only: bool diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index a67618df..b8955c48 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -1,4 +1,5 @@ import asyncio +import logging from contextlib import asynccontextmanager from enum import Enum, auto from importlib import import_module @@ -10,13 +11,17 @@ from redisvl.exceptions import RedisSearchError from redisvl.index import AsyncSearchIndex from redisvl.mcp.auth import build_auth_provider, resolve_auth_config -from redisvl.mcp.config import MCPConfig, load_mcp_config +from redisvl.mcp.config import MCPConfig, MCPIndexBindingConfig, load_mcp_config +from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError +from redisvl.mcp.runtime import BindingRuntime from redisvl.mcp.settings import MCPSettings from redisvl.mcp.tools.search import register_search_tool from redisvl.mcp.tools.upsert import register_upsert_tool from redisvl.redis.connection import RedisConnectionFactory, is_version_gte from redisvl.schema import IndexSchema +logger = logging.getLogger(__name__) + try: from fastmcp import FastMCP except ImportError: @@ -47,17 +52,15 @@ class _LifecycleState(Enum): class RedisVLMCPServer(FastMCP): - """MCP server exposing RedisVL capabilities for one existing Redis index.""" + """MCP server exposing RedisVL capabilities for one or many existing indexes.""" _LifecycleState = _LifecycleState def __init__(self, settings: MCPSettings): - """Create a server shell with lazy config, index, and vectorizer state.""" + """Create a server shell with lazy config and per-binding runtime state.""" self.mcp_settings = settings self.config: MCPConfig | None = None - self._index: AsyncSearchIndex | None = None - self._vectorizer: Any | None = None - self._supports_native_hybrid_search: bool | None = None + self._bindings: dict[str, BindingRuntime] = {} self._semaphore: asyncio.Semaphore | None = None self._tools_registered = False @@ -90,12 +93,16 @@ async def startup(self) -> None: """Load config, inspect the configured index, and initialize dependencies.""" async with self._transition_lock: await self._begin_startup() - client = None try: - client = await self._initialize_runtime_resources() + await self._initialize_runtime_resources() await self._mark_running() except Exception: - await self._teardown_runtime(client) + # Fail closed: release whatever initialization built before + # marking the server stopped. This is the single teardown path + # for any post-begin failure -- binding init, tool registration, + # or a later step such as _mark_running -- so resources are + # never leaked regardless of where startup fails. + await self._teardown_runtime() await self._mark_stopped() raise @@ -112,22 +119,53 @@ async def shutdown(self) -> None: finally: await self._mark_stopped() - async def get_index(self) -> AsyncSearchIndex: - """Return the initialized async index or fail if startup has not run.""" - if self._index is None: - raise RuntimeError("MCP server has not been started") - return self._index + def resolve_binding(self, index_id: str | None) -> BindingRuntime: + """Resolve the runtime for a logical index id, honoring single-index defaults. - async def get_vectorizer(self) -> Any: - """Return the initialized vectorizer or fail if startup has not run.""" - if self.config is None: + - ``None`` with exactly one configured binding returns that binding, + preserving backward-compatible single-index behavior. + - ``None`` with multiple bindings is an ``invalid_request``; the caller + must name an index. + - An unknown id is an ``invalid_request``. + + Write-availability is not enforced here; that is the upsert tool's job. + """ + if not self._bindings: raise RuntimeError("MCP server has not been started") - if self._vectorizer is None: - raise RuntimeError("MCP server vectorizer is not configured") - return self._vectorizer - async def run_guarded(self, operation_name: str, awaitable: Awaitable[Any]) -> Any: - """Run a coroutine under the configured concurrency and timeout limits.""" + if index_id is None: + if len(self._bindings) == 1: + return next(iter(self._bindings.values())) + available = ", ".join(sorted(self._bindings)) + raise RedisVLMCPError( + "index is required when multiple indexes are configured; " + f"available: {available}", + code=MCPErrorCode.INVALID_REQUEST, + retryable=False, + ) + + runtime = self._bindings.get(index_id) + if runtime is None: + available = ", ".join(sorted(self._bindings)) + raise RedisVLMCPError( + f"Unknown index '{index_id}'; available: {available}", + code=MCPErrorCode.INVALID_REQUEST, + retryable=False, + ) + return runtime + + async def run_guarded( + self, + operation_name: str, + awaitable: Awaitable[Any], + *, + timeout_seconds: float, + ) -> Any: + """Run a coroutine under the global concurrency cap and a request timeout. + + The timeout is sourced per-binding by the caller; the concurrency + semaphore is a single process-wide ceiling shared across all bindings. + """ del operation_name semaphore = self._semaphore if semaphore is None: @@ -140,8 +178,7 @@ async def run_guarded(self, operation_name: str, awaitable: Awaitable[Any]) -> A self._close_awaitable(awaitable) raise RuntimeError("MCP server is not running") - config = self.config - if config is None: + if self.config is None: self._close_awaitable(awaitable) raise RuntimeError("MCP server is not running") @@ -149,33 +186,32 @@ async def run_guarded(self, operation_name: str, awaitable: Awaitable[Any]) -> A self._active_requests_drained.clear() try: - return await asyncio.wait_for( - awaitable, - timeout=config.runtime.request_timeout_seconds, - ) + return await asyncio.wait_for(awaitable, timeout=timeout_seconds) finally: async with self._request_state_lock: self._active_requests -= 1 if self._active_requests == 0: self._active_requests_drained.set() - def _build_vectorizer(self) -> Any: - """Instantiate the configured vectorizer class from validated config.""" - if self.config is None: - raise RuntimeError("MCP server config not loaded") - if self.config.vectorizer is None: + @staticmethod + def _build_vectorizer(binding: MCPIndexBindingConfig) -> Any: + """Instantiate a binding's configured vectorizer class from its config.""" + if binding.vectorizer is None: raise RuntimeError("MCP server vectorizer is not configured") - vectorizer_class = resolve_vectorizer_class(self.config.vectorizer.class_name) - return vectorizer_class(**self.config.vectorizer.to_init_kwargs()) + vectorizer_class = resolve_vectorizer_class(binding.vectorizer.class_name) + return vectorizer_class(**binding.vectorizer.to_init_kwargs()) - def _validate_vectorizer_dims(self, schema: IndexSchema) -> None: + @staticmethod + def _validate_vectorizer_dims( + binding: MCPIndexBindingConfig, vectorizer: Any, schema: IndexSchema + ) -> None: """Fail startup when vectorizer dimensions disagree with schema dimensions.""" - if self.config is None or self._vectorizer is None: + if vectorizer is None: return - configured_dims = self.config.get_vector_field_dims(schema) - actual_dims = getattr(self._vectorizer, "dims", None) + configured_dims = binding.get_vector_field_dims(schema) + actual_dims = getattr(vectorizer, "dims", None) if ( configured_dims is not None and actual_dims is not None @@ -185,33 +221,32 @@ def _validate_vectorizer_dims(self, schema: IndexSchema) -> None: f"Vectorizer dims {actual_dims} do not match configured vector field dims {configured_dims}" ) - async def supports_native_hybrid_search(self) -> bool: - """Return whether the current runtime supports Redis native hybrid search.""" - if self._supports_native_hybrid_search is not None: - return self._supports_native_hybrid_search - if self._index is None: - raise RuntimeError("MCP server has not been started") + @staticmethod + async def _probe_native_hybrid_search(index: AsyncSearchIndex) -> bool: + """Probe whether a connected index supports Redis native hybrid search.""" if not is_version_gte(redis_py_version, "7.1.0"): - self._supports_native_hybrid_search = False return False - client = await self._index._get_client() + client = await index._get_client() info = await client.info("server") if not is_version_gte(info.get("redis_version", "0.0.0"), "8.4.0"): - self._supports_native_hybrid_search = False return False - self._supports_native_hybrid_search = hasattr( - client.ft(self._index.schema.index.name), "hybrid_search" - ) - return self._supports_native_hybrid_search + return hasattr(client.ft(index.schema.index.name), "hybrid_search") - def _register_tools(self, schema: IndexSchema) -> None: - """Register MCP tools once the server is ready.""" + def _register_tools(self) -> None: + """Register MCP tools once every binding is ready.""" if self._tools_registered or not hasattr(self, "tool"): return - register_search_tool(self, schema) + # The search description advertises schema-specific filter hints, which + # are only unambiguous for a single binding. With multiple bindings the + # caller selects an index per call, so fall back to the base description. + search_schema: IndexSchema | None = None + if len(self._bindings) == 1: + search_schema = next(iter(self._bindings.values())).schema + + register_search_tool(self, search_schema) if not self.mcp_settings.read_only: register_upsert_tool(self) self._tools_registered = True @@ -241,15 +276,15 @@ async def _server_lifespan(self, _server: Any): finally: await self.shutdown() - async def _teardown_runtime(self, client: Any | None = None) -> None: - """Release runtime resources and clear terminal state.""" - vectorizer = self._vectorizer - index = self._index - self._vectorizer = None - self._index = None - self.config = None - self._semaphore = None + @staticmethod + async def _close_resources( + *, index: Any | None, vectorizer: Any | None, client: Any | None = None + ) -> None: + """Close one binding's vectorizer and Redis connection. + A fully built binding owns its client through ``index``; a binding that + failed mid-startup may have a bare ``client`` and no index yet. + """ try: if vectorizer is not None: aclose = getattr(vectorizer, "aclose", None) @@ -259,12 +294,37 @@ async def _teardown_runtime(self, client: Any | None = None) -> None: elif callable(close): close() finally: - self._supports_native_hybrid_search = None if index is not None: await index.disconnect() elif client is not None: await client.aclose() + async def _teardown_runtime(self) -> None: + """Release every binding's runtime resources and clear terminal state. + + ``_tools_registered`` is intentionally *not* reset here: MCP tools are + registered once on the FastMCP instance and their closures resolve the + live binding at call time, so they survive teardown and remain valid + across a stop/start. Resetting it would make a restart re-register the + same tool names on the instance. + """ + bindings = list(self._bindings.values()) + self._bindings = {} + self.config = None + self._semaphore = None + + for runtime in bindings: + try: + await self._close_resources( + index=runtime.index, vectorizer=runtime.vectorizer + ) + except Exception: + logger.warning( + "error closing binding %s during teardown", + runtime.binding_id, + exc_info=True, + ) + @staticmethod def _close_awaitable(awaitable: Awaitable[Any]) -> None: """Close coroutine objects we reject before awaiting to avoid warnings.""" @@ -296,6 +356,7 @@ async def _begin_shutdown(self) -> bool: ): self.config = None self._semaphore = None + self._bindings = {} self._lifecycle_state = _LifecycleState.STOPPED return True @@ -329,29 +390,62 @@ def _verify_auth_not_stale(self) -> None: "ensure the config file exists before constructing the server." ) - async def _initialize_runtime_resources(self) -> Any: - """Load config and initialize the Redis-backed runtime dependencies.""" + async def _initialize_runtime_resources(self) -> None: + """Load config and initialize every configured binding independently.""" self.config = load_mcp_config(self._config_path) self._verify_auth_not_stale() - self._semaphore = asyncio.Semaphore(self.config.runtime.max_concurrency) - self._supports_native_hybrid_search = None - timeout = self.config.runtime.startup_timeout_seconds + # The semaphore is a single process-wide concurrency ceiling shared by + # all bindings; take the max across bindings. This means the most + # permissive binding sets the cap β€” e.g. five bindings each configured + # with max_concurrency=2 yield Semaphore(2), not Semaphore(10). + self._semaphore = asyncio.Semaphore( + max( + binding.runtime.max_concurrency + for binding in self.config.indexes.values() + ) + ) + self._bindings = {} + + # On failure, startup()'s handler tears down any bindings built here, so + # this method does not need its own teardown. (A binding that fails + # mid-build closes its own bare client inside _initialize_binding.) + for binding_id, binding in self.config.indexes.items(): + self._bindings[binding_id] = await self._initialize_binding( + binding_id, binding + ) + self._register_tools() + async def _initialize_binding( + self, binding_id: str, binding: MCPIndexBindingConfig + ) -> BindingRuntime: + """Inspect, validate, and initialize a single configured binding.""" + timeout = binding.runtime.startup_timeout_seconds client = await self._connect_redis_client(timeout) + index: AsyncSearchIndex | None = None + vectorizer: Any | None = None try: - effective_schema = await self._load_effective_schema(client, timeout) - self._initialize_index(effective_schema, client) - self.config.validate_search( - schema=effective_schema, - supports_native_hybrid_search=await self.supports_native_hybrid_search(), + schema = await self._load_effective_schema(binding, client, timeout) + index = self._make_index(schema, client) + supports_native_hybrid = await self._probe_native_hybrid_search(index) + binding.validate_search( + schema=schema, + supports_native_hybrid_search=supports_native_hybrid, + ) + if binding.requires_startup_vectorizer: + vectorizer = await self._initialize_vectorizer(binding, schema, timeout) + return BindingRuntime( + binding_id=binding_id, + binding=binding, + index=index, + schema=schema, + vectorizer=vectorizer, + supports_native_hybrid_search=supports_native_hybrid, + effective_read_only=self.mcp_settings.read_only or binding.read_only, ) - if self.config.requires_startup_vectorizer: - await self._initialize_vectorizer(effective_schema, timeout) - self._register_tools(effective_schema) - return client except Exception: - if self._index is None: - await client.aclose() + await self._close_resources( + index=index, vectorizer=vectorizer, client=client + ) raise async def _connect_redis_client(self, timeout: int) -> Any: @@ -368,37 +462,41 @@ async def _connect_redis_client(self, timeout: int) -> Any: await asyncio.wait_for(client.info("server"), timeout=timeout) return client - async def _load_effective_schema(self, client: Any, timeout: int) -> IndexSchema: - """Inspect the configured Redis index and build the effective schema.""" - if self.config is None: - raise RuntimeError("MCP server config not loaded") - + async def _load_effective_schema( + self, binding: MCPIndexBindingConfig, client: Any, timeout: int + ) -> IndexSchema: + """Inspect a binding's Redis index and build its effective schema.""" try: index_info = await asyncio.wait_for( - AsyncSearchIndex._info(self.config.redis_name, client), + AsyncSearchIndex._info(binding.redis_name, client), timeout=timeout, ) except RedisSearchError as exc: if self._is_missing_index_error(exc): raise ValueError( - f"Configured Redis index '{self.config.redis_name}' does not exist" + f"Configured Redis index '{binding.redis_name}' does not exist" ) from exc raise - inspected_schema = self.config.inspected_schema_from_index_info(index_info) - return self.config.to_index_schema(inspected_schema) + inspected_schema = binding.inspected_schema_from_index_info(index_info) + return binding.to_index_schema(inspected_schema) - def _initialize_index(self, schema: IndexSchema, client: Any) -> None: - """Bind the inspected schema and Redis client into an async index.""" - self._index = AsyncSearchIndex(schema=schema, redis_client=client) + @staticmethod + def _make_index(schema: IndexSchema, client: Any) -> AsyncSearchIndex: + """Bind an inspected schema and Redis client into an async index.""" + index = AsyncSearchIndex(schema=schema, redis_client=client) # The server acquired this client explicitly during startup, so hand # ownership to the index for a single shutdown path. - self._index._owns_redis_client = True - - async def _initialize_vectorizer(self, schema: IndexSchema, timeout: int) -> None: - """Build the configured vectorizer and validate it against the schema.""" - self._vectorizer = await asyncio.wait_for( - asyncio.to_thread(self._build_vectorizer), + index._owns_redis_client = True + return index + + async def _initialize_vectorizer( + self, binding: MCPIndexBindingConfig, schema: IndexSchema, timeout: int + ) -> Any: + """Build a binding's vectorizer and validate it against the schema.""" + vectorizer = await asyncio.wait_for( + asyncio.to_thread(self._build_vectorizer, binding), timeout=timeout, ) - self._validate_vectorizer_dims(schema) + self._validate_vectorizer_dims(binding, vectorizer, schema) + return vectorizer diff --git a/redisvl/mcp/tools/search.py b/redisvl/mcp/tools/search.py index 89cfa00f..36d136f7 100644 --- a/redisvl/mcp/tools/search.py +++ b/redisvl/mcp/tools/search.py @@ -51,10 +51,17 @@ def _build_return_fields_hint(schema: IndexSchema) -> str: def _build_search_tool_description( - schema: IndexSchema, base_description: str | None = None + schema: IndexSchema | None, base_description: str | None = None ) -> str: - """Build the `search-records` description from static text plus schema hints.""" + """Build the `search-records` description from static text plus schema hints. + + With multiple bindings configured the schema is ambiguous (the caller picks + an index per call via `list-indexes`), so `schema` is None and only the + base description is returned. + """ description = (base_description or DEFAULT_SEARCH_DESCRIPTION).strip() + if schema is None: + return description # `exists` is currently accepted for any schema field in the MCP object filter. exists_fields = [field.name for field in schema.fields.values()] @@ -79,18 +86,16 @@ def _validate_request( limit: int | None, offset: int, return_fields: list[str] | None, - server: Any, - index: Any, + runtime: Any, + schema: Any, ) -> tuple[int, list[str]]: """Validate a `search-records` request and resolve default projection. The MCP caller can only supply query text, pagination, filters, and return - fields. Search mode and tuning are sourced from config, so this validation - step focuses only on the public request contract. + fields. Search mode and tuning are sourced from the selected binding's + config, so this validation step focuses only on the public request contract. """ - runtime = server.config.runtime - if not isinstance(query, str) or not query.strip(): raise RedisVLMCPError( "query must be a non-empty string", @@ -125,17 +130,17 @@ def _validate_request( retryable=False, ) - schema_fields = set(index.schema.field_names) + schema_fields = set(schema.field_names) vector_field_names = { field_name - for field_name, field in index.schema.fields.items() + for field_name, field in schema.fields.items() if field.type == "vector" } if return_fields is None: fields = [ field_name - for field_name in index.schema.field_names + for field_name in schema.field_names if field_name not in vector_field_names ] else: @@ -224,12 +229,19 @@ async def _embed_query(vectorizer: Any, query: str) -> Any: return await asyncio.to_thread(embed, query) -def _get_configured_search(server: Any) -> tuple[str, dict[str, Any]]: - """Return the configured search mode and normalized query params.""" - search_config = server.config.search +def _get_configured_search(rt: Any) -> tuple[str, dict[str, Any]]: + """Return the binding's configured search mode and normalized query params.""" + search_config = rt.binding.search return search_config.type, search_config.to_query_params() +def _require_vectorizer(rt: Any) -> Any: + """Return the binding's vectorizer or fail when it is not configured.""" + if rt.vectorizer is None: + raise RuntimeError("MCP server vectorizer is not configured") + return rt.vectorizer + + def _build_native_hybrid_kwargs( *, query: str, @@ -311,29 +323,27 @@ def _build_fallback_hybrid_kwargs( async def _build_query( *, - server: Any, - index: Any, + rt: Any, query: str, limit: int, offset: int, filter_value: str | dict[str, Any] | None, return_fields: list[str], ) -> tuple[Any, str, str, str]: - """Build the RedisVL query object from configured search mode and params. + """Build the RedisVL query object from the binding's search mode and params. Returns the query instance, the raw score field to read from RedisVL results, the public MCP `score_type`, and the configured `search_type`. """ - runtime = server.config.runtime - search_type, search_params = _get_configured_search(server) + runtime = rt.binding.runtime + search_type, search_params = _get_configured_search(rt) num_results = limit + offset - filter_expression = parse_filter(filter_value, index.schema) + filter_expression = parse_filter(filter_value, rt.schema) if search_type == "vector": if runtime.vector_field_name is None: raise RuntimeError("Vector search requires a configured vector field") - vectorizer = await server.get_vectorizer() - embedding = await _embed_query(vectorizer, query) + embedding = await _embed_query(_require_vectorizer(rt), query) vector_kwargs = { "vector": embedding, "vector_field_name": runtime.vector_field_name, @@ -373,9 +383,8 @@ async def _build_query( search_type, ) - vectorizer = await server.get_vectorizer() - embedding = await _embed_query(vectorizer, query) - if await server.supports_native_hybrid_search(): + embedding = await _embed_query(_require_vectorizer(rt), query) + if rt.supports_native_hybrid_search: native_query = HybridQuery( **_build_native_hybrid_kwargs( query=query, @@ -423,20 +432,19 @@ async def search_records( filter: str | dict[str, Any] | None = None, return_fields: list[str] | None = None, ) -> dict[str, Any]: - """Execute `search-records` against the configured Redis index binding.""" + """Execute `search-records` against the selected Redis index binding.""" try: - index = await server.get_index() + rt = server.resolve_binding(None) effective_limit, effective_return_fields = _validate_request( query=query, limit=limit, offset=offset, return_fields=return_fields, - server=server, - index=index, + runtime=rt.binding.runtime, + schema=rt.schema, ) built_query, score_field, score_type, search_type = await _build_query( - server=server, - index=index, + rt=rt, query=query.strip(), limit=effective_limit, offset=offset, @@ -445,7 +453,8 @@ async def search_records( ) raw_results = await server.run_guarded( "search-records", - index.query(built_query), + rt.index.query(built_query), + timeout_seconds=rt.binding.runtime.request_timeout_seconds, ) sliced_results = raw_results[offset : offset + effective_limit] return { @@ -467,7 +476,7 @@ async def search_records( raise map_exception(exc) from exc -def register_search_tool(server: Any, schema: IndexSchema) -> None: +def register_search_tool(server: Any, schema: IndexSchema | None) -> None: """Register the MCP `search-records` tool with its config-owned contract.""" description = _build_search_tool_description( schema=schema, @@ -482,7 +491,8 @@ async def search_records_tool( return_fields: list[str] | None = None, ): """FastMCP wrapper for the `search-records` tool.""" - read_scope = getattr(getattr(server, "auth_config", None), "read_scope", None) + auth_config = getattr(server, "auth_config", None) + read_scope = auth_config.read_scope if auth_config is not None else None ensure_tool_scope(server, read_scope) return await search_records( server, diff --git a/redisvl/mcp/tools/upsert.py b/redisvl/mcp/tools/upsert.py index dd5721e8..c0d3b7cc 100644 --- a/redisvl/mcp/tools/upsert.py +++ b/redisvl/mcp/tools/upsert.py @@ -14,14 +14,12 @@ def _validate_request( *, - server: Any, + runtime: Any, records: list[dict[str, Any]], id_field: str | None, skip_embedding_if_present: bool | None, ) -> bool: """Validate the public upsert request contract and resolve defaults.""" - runtime = server.config.runtime - if not isinstance(records, list) or not records: raise RedisVLMCPError( "records must be a non-empty list", @@ -183,9 +181,9 @@ async def _embed_many(vectorizer: Any, contents: list[str]) -> list[list[float]] return embeddings -def _vector_dtype(server: Any, index: Any) -> str: - """Resolve the configured vector field datatype as a lowercase string.""" - field = server.config.get_vector_field(index.schema) +def _vector_dtype(rt: Any) -> str: + """Resolve the binding's vector field datatype as a lowercase string.""" + field = rt.binding.get_vector_field(rt.schema) datatype = getattr(field.attrs.datatype, "value", field.attrs.datatype) return str(datatype).lower() @@ -225,12 +223,12 @@ def _validate_record( def _prepare_record_for_storage( record: dict[str, Any], *, - server: Any, - index: Any, + rt: Any, ) -> dict[str, Any]: """Validate records before serializing HASH vectors for storage.""" prepared = dict(record) - vector_field_name = server.config.runtime.vector_field_name + index = rt.index + vector_field_name = rt.binding.runtime.vector_field_name _validate_record(prepared, index=index, vector_field_name=vector_field_name) if vector_field_name is None: @@ -242,7 +240,7 @@ def _prepare_record_for_storage( if isinstance(vector_value, list): prepared[vector_field_name] = array_to_buffer( vector_value, - _vector_dtype(server, index), + _vector_dtype(rt), ) return prepared @@ -254,11 +252,19 @@ async def upsert_records( id_field: str | None = None, skip_embedding_if_present: bool | None = None, ) -> dict[str, Any]: - """Execute `upsert-records` against the configured Redis index.""" + """Execute `upsert-records` against the selected Redis index binding.""" try: - index = await server.get_index() + rt = server.resolve_binding(None) + if rt.effective_read_only: + raise RedisVLMCPError( + "upsert-records is not permitted: binding is read-only", + code=MCPErrorCode.FORBIDDEN, + retryable=False, + ) + index = rt.index + runtime = rt.binding.runtime effective_skip_embedding = _validate_request( - server=server, + runtime=runtime, records=records, id_field=id_field, skip_embedding_if_present=skip_embedding_if_present, @@ -266,14 +272,13 @@ async def upsert_records( # Copy caller-provided records before enriching them with embeddings or # storage-specific serialization so the MCP tool does not mutate inputs. prepared_records = [deepcopy(record) for record in records] - runtime = server.config.runtime for record in prepared_records: _validate_record( record, index=index, vector_field_name=runtime.vector_field_name, ) - if server.config.supports_server_side_embedding: + if rt.binding.supports_server_side_embedding: if ( runtime.default_embed_text_field is None or runtime.vector_field_name is None @@ -289,7 +294,9 @@ async def upsert_records( ) if embed_contents: - vectorizer = await server.get_vectorizer() + if rt.vectorizer is None: + raise RuntimeError("MCP server vectorizer is not configured") + vectorizer = rt.vectorizer # TODO: Avoid re-embedding records that already include vectors. # The current flow can regenerate embeddings for caller-supplied # vectors, which is wasteful and can add external service cost. @@ -319,14 +326,14 @@ async def upsert_records( ) loadable_records = [ - _prepare_record_for_storage(record, server=server, index=index) - for record in prepared_records + _prepare_record_for_storage(record, rt=rt) for record in prepared_records ] try: keys = await server.run_guarded( "upsert-records", index.load(loadable_records, id_field=id_field), + timeout_seconds=runtime.request_timeout_seconds, ) except Exception as exc: mapped = map_exception(exc) @@ -356,7 +363,8 @@ async def upsert_records_tool( skip_embedding_if_present: bool | None = None, ): """FastMCP wrapper for the `upsert-records` tool.""" - write_scope = getattr(getattr(server, "auth_config", None), "write_scope", None) + auth_config = getattr(server, "auth_config", None) + write_scope = auth_config.write_scope if auth_config is not None else None ensure_tool_scope(server, write_scope) return await upsert_records( server, diff --git a/tests/conftest.py b/tests/conftest.py index c3ac61ba..cd09ef5d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -705,6 +705,29 @@ def hash_preprocess(item: dict) -> dict: index.delete(drop=True) +# MCP server test helpers +def mcp_binding_index(server, index_id=None): + """Return a started MCP server's resolved index for the given binding. + + Test-only replacement for the removed ``get_index`` convenience: it routes + through ``resolve_binding`` so it preserves the same ``index is required`` / + ``has not been started`` errors the real tools see. + """ + return server.resolve_binding(index_id).index + + +def mcp_binding_vectorizer(server, index_id=None): + """Return a started MCP server's resolved vectorizer for the given binding. + + Test-only replacement for the removed ``get_vectorizer`` convenience, + mirroring its "vectorizer is not configured" error when none is set. + """ + runtime = server.resolve_binding(index_id) + if runtime.vectorizer is None: + raise RuntimeError("MCP server vectorizer is not configured") + return runtime.vectorizer + + # Version checking utilities def get_redis_version(client): """Get Redis version from client info.""" diff --git a/tests/integration/test_mcp/test_server_startup.py b/tests/integration/test_mcp/test_server_startup.py index 6b9c315a..c278e57e 100644 --- a/tests/integration/test_mcp/test_server_startup.py +++ b/tests/integration/test_mcp/test_server_startup.py @@ -6,11 +6,16 @@ import yaml from redisvl.index import AsyncSearchIndex +from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError from redisvl.mcp.server import RedisVLMCPServer from redisvl.mcp.settings import MCPSettings from redisvl.redis.connection import is_version_gte from redisvl.schema import IndexSchema -from tests.conftest import get_redis_version_async +from tests.conftest import ( + get_redis_version_async, + mcp_binding_index, + mcp_binding_vectorizer, +) class FakeVectorizer: @@ -133,8 +138,8 @@ async def test_server_startup_success(monkeypatch, existing_index, mcp_config_pa await server.startup() - started_index = await server.get_index() - vectorizer = await server.get_vectorizer() + started_index = mcp_binding_index(server) + vectorizer = mcp_binding_vectorizer(server) assert await started_index.exists() is True assert started_index.schema.index.name == index.name @@ -154,10 +159,10 @@ async def test_server_startup_succeeds_for_fulltext_without_vectorizer( original_build_vectorizer = RedisVLMCPServer._build_vectorizer build_vectorizer_called = False - def tracked_build_vectorizer(self): + def tracked_build_vectorizer(binding): nonlocal build_vectorizer_called build_vectorizer_called = True - return original_build_vectorizer(self) + return original_build_vectorizer(binding) monkeypatch.setattr( "redisvl.mcp.server.resolve_vectorizer_class", @@ -166,13 +171,13 @@ def tracked_build_vectorizer(self): monkeypatch.setattr( RedisVLMCPServer, "_build_vectorizer", - tracked_build_vectorizer, + staticmethod(tracked_build_vectorizer), ) server = RedisVLMCPServer( MCPSettings( config=mcp_config_path( redis_name=index.name, - search={"type": "fulltext"}, + search={"type": "fulltext", "params": {"stopwords": None}}, runtime_overrides={ "vector_field_name": None, "default_embed_text_field": None, @@ -184,11 +189,11 @@ def tracked_build_vectorizer(self): await server.startup() - started_index = await server.get_index() + started_index = mcp_binding_index(server) assert await started_index.exists() is True assert build_vectorizer_called is False with pytest.raises(RuntimeError, match="vectorizer is not configured"): - await server.get_vectorizer() + mcp_binding_vectorizer(server) await server.shutdown() @@ -213,6 +218,7 @@ async def test_server_fails_when_hybrid_config_requires_native_runtime( search={ "type": "hybrid", "params": { + "stopwords": None, "vector_search_method": "KNN", "knn_ef_runtime": 150, }, @@ -287,7 +293,7 @@ async def incomplete_info(name, redis_client): await server.startup() - started_index = await server.get_index() + started_index = mcp_binding_index(server) assert started_index.schema.fields["embedding"].attrs.dims == 3 await server.shutdown() @@ -389,7 +395,7 @@ async def test_server_shutdown_disconnects_owned_client( ) await server.startup() - started_index = await server.get_index() + started_index = mcp_binding_index(server) assert started_index.client is not None @@ -415,7 +421,7 @@ async def test_server_get_index_fails_after_shutdown( await server.shutdown() with pytest.raises(RuntimeError, match="has not been started"): - await server.get_index() + mcp_binding_index(server) @pytest.mark.asyncio @@ -432,15 +438,17 @@ async def test_server_shutdown_disconnects_index_when_vectorizer_close_fails( ) await server.startup() - started_index = await server.get_index() + started_index = mcp_binding_index(server) - with pytest.raises(RuntimeError, match="vectorizer close failed"): - await server.shutdown() + # Teardown is best-effort: a failing vectorizer close is logged and + # swallowed rather than aborting teardown, so the index is still + # disconnected and its Redis connection cannot leak. + await server.shutdown() assert started_index.client is None with pytest.raises(RuntimeError, match="has not been started"): - await server.get_vectorizer() + mcp_binding_vectorizer(server) @pytest.mark.asyncio @@ -462,7 +470,7 @@ async def test_run_guarded_allows_admitted_request_to_finish_during_shutdown( ) await server.startup() - started_index = await server.get_index() + started_index = mcp_binding_index(server) entered = asyncio.Event() release = asyncio.Event() @@ -472,7 +480,9 @@ async def guarded_operation(): return "done" operation_task = asyncio.create_task( - server.run_guarded("drain-during-shutdown", guarded_operation()) + server.run_guarded( + "drain-during-shutdown", guarded_operation(), timeout_seconds=5 + ) ) await entered.wait() @@ -518,7 +528,9 @@ async def guarded_operation(): return "done" active_task = asyncio.create_task( - server.run_guarded("active-during-shutdown", guarded_operation()) + server.run_guarded( + "active-during-shutdown", guarded_operation(), timeout_seconds=5 + ) ) await entered.wait() @@ -528,7 +540,7 @@ async def guarded_operation(): future = asyncio.get_running_loop().create_future() future.set_result("later") with pytest.raises(RuntimeError, match="not running"): - await server.run_guarded("reject-after-stop", future) + await server.run_guarded("reject-after-stop", future, timeout_seconds=5) release.set() assert await active_task == "done" @@ -567,11 +579,13 @@ async def second_operation(): second_started.set() return "second" - first_task = asyncio.create_task(server.run_guarded("first-op", first_operation())) + first_task = asyncio.create_task( + server.run_guarded("first-op", first_operation(), timeout_seconds=5) + ) await first_entered.wait() second_task = asyncio.create_task( - server.run_guarded("second-op", second_operation()) + server.run_guarded("second-op", second_operation(), timeout_seconds=5) ) await asyncio.sleep(0) @@ -586,3 +600,133 @@ async def second_operation(): assert second_started.is_set() is False await shutdown_task + + +@pytest.fixture +def multi_index_config_path(tmp_path: Path, redis_url: str): + def factory(bindings: dict[str, dict[str, Any]]) -> str: + config = {"server": {"redis_url": redis_url}, "indexes": bindings} + config_path = tmp_path / "multi-index.yaml" + config_path.write_text(yaml.safe_dump(config), encoding="utf-8") + return str(config_path) + + return factory + + +def _binding_config(redis_name: str, *, read_only: bool = False) -> dict[str, Any]: + return { + "redis_name": redis_name, + "read_only": read_only, + "vectorizer": {"class": "FakeVectorizer", "model": "fake-model", "dims": 3}, + "search": {"type": "vector"}, + "runtime": { + "text_field_name": "content", + "vector_field_name": "embedding", + "default_embed_text_field": "content", + }, + } + + +@pytest.mark.asyncio +async def test_server_starts_with_multiple_bindings( + monkeypatch, existing_index, multi_index_config_path +): + knowledge = await existing_index(index_name="mcp-multi-knowledge") + tickets = await existing_index(index_name="mcp-multi-tickets") + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + server = RedisVLMCPServer( + MCPSettings( + config=multi_index_config_path( + { + "knowledge": _binding_config(knowledge.name), + "tickets": _binding_config(tickets.name, read_only=True), + } + ) + ) + ) + + await server.startup() + + try: + assert sorted(server._bindings) == ["knowledge", "tickets"] + + knowledge_rt = server.resolve_binding("knowledge") + tickets_rt = server.resolve_binding("tickets") + + # Each binding is inspected and initialized independently. + assert knowledge_rt.index.schema.index.name == knowledge.name + assert tickets_rt.index.schema.index.name == tickets.name + assert knowledge_rt.index is not tickets_rt.index + + # Per-index write availability is respected. + assert knowledge_rt.effective_read_only is False + assert tickets_rt.effective_read_only is True + + # An omitted index is ambiguous when multiple bindings are configured. + with pytest.raises(RedisVLMCPError) as excinfo: + server.resolve_binding(None) + assert excinfo.value.code == MCPErrorCode.INVALID_REQUEST + finally: + await server.shutdown() + + +@pytest.mark.asyncio +async def test_server_global_read_only_overrides_all_bindings( + monkeypatch, existing_index, multi_index_config_path +): + knowledge = await existing_index(index_name="mcp-multi-ro-knowledge") + tickets = await existing_index(index_name="mcp-multi-ro-tickets") + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + server = RedisVLMCPServer( + MCPSettings( + config=multi_index_config_path( + { + "knowledge": _binding_config(knowledge.name), + "tickets": _binding_config(tickets.name, read_only=True), + } + ), + read_only=True, + ) + ) + + await server.startup() + + try: + # Global read-only forces effective write availability false everywhere. + assert server.resolve_binding("knowledge").effective_read_only is True + assert server.resolve_binding("tickets").effective_read_only is True + finally: + await server.shutdown() + + +@pytest.mark.asyncio +async def test_server_startup_fails_when_one_binding_is_invalid( + monkeypatch, existing_index, multi_index_config_path +): + knowledge = await existing_index(index_name="mcp-multi-invalid") + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + server = RedisVLMCPServer( + MCPSettings( + config=multi_index_config_path( + { + "knowledge": _binding_config(knowledge.name), + "missing": _binding_config("nonexistent-index-name"), + } + ) + ) + ) + + with pytest.raises(ValueError, match="does not exist"): + await server.startup() + + assert server._lifecycle_state.name == "STOPPED" + assert server._bindings == {} diff --git a/tests/integration/test_mcp/test_transport_auth.py b/tests/integration/test_mcp/test_transport_auth.py index a31d84f7..9c2161e6 100644 --- a/tests/integration/test_mcp/test_transport_auth.py +++ b/tests/integration/test_mcp/test_transport_auth.py @@ -107,7 +107,7 @@ def factory(redis_name: str, public_key: str) -> str: "indexes": { "knowledge": { "redis_name": redis_name, - "search": {"type": "fulltext"}, + "search": {"type": "fulltext", "params": {"stopwords": None}}, "runtime": {"text_field_name": "content"}, } }, @@ -242,7 +242,7 @@ async def test_http_transport_gates_by_roles_claim( "indexes": { "knowledge": { "redis_name": auth_index.schema.index.name, - "search": {"type": "fulltext"}, + "search": {"type": "fulltext", "params": {"stopwords": None}}, "runtime": {"text_field_name": "content"}, } }, diff --git a/tests/integration/test_mcp/test_upsert_tool.py b/tests/integration/test_mcp/test_upsert_tool.py index 6711790b..ec08d358 100644 --- a/tests/integration/test_mcp/test_upsert_tool.py +++ b/tests/integration/test_mcp/test_upsert_tool.py @@ -10,6 +10,7 @@ from redisvl.mcp.settings import MCPSettings from redisvl.mcp.tools.upsert import upsert_records from redisvl.schema import IndexSchema +from tests.conftest import mcp_binding_vectorizer class RecordingVectorizer: @@ -224,7 +225,7 @@ async def test_upsert_records_inserts_rows_into_hash_index( assert response["keys_upserted"] == 2 assert len(response["keys"]) == 2 - vectorizer = await server.get_vectorizer() + vectorizer = mcp_binding_vectorizer(server) assert vectorizer.aembed_many_inputs == [ ["first upserted document", "second upserted document"] ] @@ -241,7 +242,7 @@ async def test_upsert_records_supports_plain_writes_without_vector_configuration ): server = await started_server( redis_name=fulltext_only_upsert_index.schema.index.name, - search={"type": "fulltext"}, + search={"type": "fulltext", "params": {"stopwords": None}}, runtime_overrides={ "vector_field_name": None, "default_embed_text_field": None, @@ -269,7 +270,7 @@ async def test_upsert_records_requires_vectors_when_embedding_is_disabled( started_server, ): server = await started_server( - search={"type": "fulltext"}, + search={"type": "fulltext", "params": {"stopwords": None}}, runtime_overrides={"default_embed_text_field": None}, include_vectorizer=False, ) diff --git a/tests/unit/test_mcp/test_config.py b/tests/unit/test_mcp/test_config.py index 7631e368..512af828 100644 --- a/tests/unit/test_mcp/test_config.py +++ b/tests/unit/test_mcp/test_config.py @@ -90,11 +90,12 @@ def test_load_mcp_config_env_substitution(tmp_path: Path, monkeypatch): config = load_mcp_config(str(config_path)) assert config.server.redis_url == "redis://localhost:6379" - assert config.binding_id == "knowledge" - assert config.redis_name == "docs-index" - assert config.vectorizer.class_name == "FakeVectorizer" - assert config.vectorizer.model == "test-model" - assert config.vectorizer.extra_kwargs == {"api_config": {"api_key": "secret"}} + assert list(config.indexes) == ["knowledge"] + binding = config.indexes["knowledge"] + assert binding.redis_name == "docs-index" + assert binding.vectorizer.class_name == "FakeVectorizer" + assert binding.vectorizer.model == "test-model" + assert binding.vectorizer.extra_kwargs == {"api_config": {"api_key": "secret"}} def test_load_mcp_config_required_env_missing(tmp_path: Path, monkeypatch): @@ -132,24 +133,46 @@ def test_mcp_config_requires_server_redis_url(): MCPConfig.model_validate(config) -@pytest.mark.parametrize( - "indexes", - [ - {}, - { - "knowledge": deepcopy(_valid_config()["indexes"]["knowledge"]), - "other": deepcopy(_valid_config()["indexes"]["knowledge"]), - }, - ], -) -def test_mcp_config_validates_index_count(indexes): +def test_mcp_config_requires_at_least_one_binding(): config = _valid_config() - config["indexes"] = indexes + config["indexes"] = {} - with pytest.raises(ValueError, match="exactly one configured index binding"): + with pytest.raises(ValueError, match="at least one configured binding"): MCPConfig.model_validate(config) +def test_mcp_config_allows_multiple_bindings(): + config = _valid_config() + config["indexes"] = { + "knowledge": deepcopy(_valid_config()["indexes"]["knowledge"]), + "tickets": deepcopy(_valid_config()["indexes"]["knowledge"]), + } + + loaded = MCPConfig.model_validate(config) + + assert list(loaded.indexes) == ["knowledge", "tickets"] + assert loaded.indexes["tickets"].redis_name == "docs-index" + + +def test_mcp_config_binding_defaults_for_description_and_read_only(): + config = MCPConfig.model_validate(_valid_config()) + + binding = config.indexes["knowledge"] + assert binding.description is None + assert binding.read_only is False + + +def test_mcp_config_binding_accepts_description_and_read_only(): + config = _valid_config() + config["indexes"]["knowledge"]["description"] = "Product docs and runbooks" + config["indexes"]["knowledge"]["read_only"] = True + + binding = MCPConfig.model_validate(config).indexes["knowledge"] + + assert binding.description == "Product docs and runbooks" + assert binding.read_only is True + + def test_mcp_config_rejects_blank_binding_id(): config = _valid_config() config["indexes"] = {"": deepcopy(config["indexes"]["knowledge"])} @@ -166,25 +189,24 @@ def test_mcp_config_rejects_blank_redis_name(): MCPConfig.model_validate(config) -def test_mcp_config_binding_helpers(): +def test_mcp_config_binding_exposes_index_settings(): config = MCPConfig.model_validate(_valid_config()) - assert config.binding_id == "knowledge" - assert config.binding.redis_name == "docs-index" - assert config.binding.search.type == "vector" - assert config.runtime.default_embed_text_field == "content" - assert config.vectorizer.class_name == "FakeVectorizer" - assert config.redis_name == "docs-index" + binding = config.indexes["knowledge"] + assert binding.redis_name == "docs-index" + assert binding.search.type == "vector" + assert binding.runtime.default_embed_text_field == "content" + assert binding.vectorizer.class_name == "FakeVectorizer" def test_vector_search_config_can_omit_text_field_name(): config = _valid_config() del config["indexes"]["knowledge"]["runtime"]["text_field_name"] - loaded = MCPConfig.model_validate(config) + binding = MCPConfig.model_validate(config).indexes["knowledge"] - assert loaded.search.type == "vector" - assert loaded.runtime.text_field_name is None + assert binding.search.type == "vector" + assert binding.runtime.text_field_name is None def test_fulltext_config_can_omit_vector_settings_and_vectorizer(): @@ -194,12 +216,12 @@ def test_fulltext_config_can_omit_vector_settings_and_vectorizer(): del config["indexes"]["knowledge"]["runtime"]["vector_field_name"] del config["indexes"]["knowledge"]["runtime"]["default_embed_text_field"] - loaded = MCPConfig.model_validate(config) + binding = MCPConfig.model_validate(config).indexes["knowledge"] - assert loaded.search.type == "fulltext" - assert loaded.vectorizer is None - assert loaded.runtime.vector_field_name is None - assert loaded.runtime.default_embed_text_field is None + assert binding.search.type == "fulltext" + assert binding.vectorizer is None + assert binding.runtime.vector_field_name is None + assert binding.runtime.default_embed_text_field is None def test_mcp_config_merges_schema_overrides_into_inspection_result(): @@ -221,7 +243,7 @@ def test_mcp_config_merges_schema_overrides_into_inspection_result(): inspected["fields"][1]["attrs"] = {"algorithm": "flat"} config = MCPConfig.model_validate(config_dict) - schema = config.to_index_schema(inspected) + schema = config.indexes["knowledge"].to_index_schema(inspected) assert isinstance(schema, IndexSchema) assert schema.index.name == "docs-index" @@ -237,7 +259,7 @@ def test_mcp_config_rejects_override_for_unknown_field(): config = MCPConfig.model_validate(config_dict) with pytest.raises(ValueError, match="schema_overrides.fields.*missing"): - config.to_index_schema(_inspected_schema()) + config.indexes["knowledge"].to_index_schema(_inspected_schema()) def test_mcp_config_rejects_override_type_conflict(): @@ -248,7 +270,7 @@ def test_mcp_config_rejects_override_type_conflict(): config = MCPConfig.model_validate(config_dict) with pytest.raises(ValueError, match="cannot change discovered field type"): - config.to_index_schema(_inspected_schema()) + config.indexes["knowledge"].to_index_schema(_inspected_schema()) def test_mcp_config_rejects_override_path_conflict(): @@ -280,7 +302,7 @@ def test_mcp_config_rejects_override_path_conflict(): config = MCPConfig.model_validate(config_dict) with pytest.raises(ValueError, match="cannot change discovered field path"): - config.to_index_schema(inspected) + config.indexes["knowledge"].to_index_schema(inspected) def test_mcp_config_validates_runtime_mapping_against_effective_schema(): @@ -289,7 +311,7 @@ def test_mcp_config_validates_runtime_mapping_against_effective_schema(): config = MCPConfig.model_validate(config_dict) with pytest.raises(ValueError, match="runtime.vector_field_name"): - config.to_index_schema(_inspected_schema()) + config.indexes["knowledge"].to_index_schema(_inspected_schema()) def test_fulltext_config_does_not_require_vector_mapping_in_schema(): @@ -300,12 +322,12 @@ def test_fulltext_config_does_not_require_vector_mapping_in_schema(): del config_dict["indexes"]["knowledge"]["runtime"]["default_embed_text_field"] config = MCPConfig.model_validate(config_dict) - schema = config.to_index_schema(_inspected_schema()) + schema = config.indexes["knowledge"].to_index_schema(_inspected_schema()) assert isinstance(schema, IndexSchema) -def test_load_mcp_config_requires_exactly_one_binding(tmp_path: Path): +def test_load_mcp_config_requires_at_least_one_binding(tmp_path: Path): config_path = tmp_path / "mcp.yaml" config_path.write_text( yaml.safe_dump( @@ -317,7 +339,7 @@ def test_load_mcp_config_requires_exactly_one_binding(tmp_path: Path): encoding="utf-8", ) - with pytest.raises(ValueError, match="exactly one configured index binding"): + with pytest.raises(ValueError, match="at least one configured binding"): load_mcp_config(str(config_path)) @@ -328,8 +350,8 @@ def test_mcp_config_accepts_search_types(search_type): loaded = MCPConfig.model_validate(config) - assert loaded.binding.search.type == search_type - assert loaded.binding.search.params == {} + assert loaded.indexes["knowledge"].search.type == search_type + assert loaded.indexes["knowledge"].search.params == {} def test_mcp_config_requires_search_type(): @@ -441,8 +463,8 @@ def test_mcp_config_normalizes_hybrid_linear_text_weight(): loaded = MCPConfig.model_validate(config) - assert loaded.binding.search.type == "hybrid" - assert loaded.binding.search.params["linear_text_weight"] == 0.3 + assert loaded.indexes["knowledge"].search.type == "hybrid" + assert loaded.indexes["knowledge"].search.params["linear_text_weight"] == 0.3 def test_mcp_config_allows_linear_text_weight_without_explicit_combination_method(): @@ -456,8 +478,8 @@ def test_mcp_config_allows_linear_text_weight_without_explicit_combination_metho loaded = MCPConfig.model_validate(config) - assert loaded.binding.search.type == "hybrid" - assert loaded.binding.search.params["linear_text_weight"] == 0.3 + assert loaded.indexes["knowledge"].search.type == "hybrid" + assert loaded.indexes["knowledge"].search.params["linear_text_weight"] == 0.3 @pytest.mark.parametrize( @@ -476,10 +498,10 @@ def test_mcp_config_rejects_native_only_hybrid_runtime_params(params): } loaded = MCPConfig.model_validate(config) - schema = loaded.to_index_schema(_inspected_schema()) + schema = loaded.indexes["knowledge"].to_index_schema(_inspected_schema()) with pytest.raises(ValueError, match="native hybrid search support"): - loaded.validate_search( + loaded.indexes["knowledge"].validate_search( schema=schema, supports_native_hybrid_search=False, ) @@ -497,9 +519,9 @@ def test_mcp_config_allows_linear_hybrid_fallback_params(): } loaded = MCPConfig.model_validate(config) - schema = loaded.to_index_schema(_inspected_schema()) + schema = loaded.indexes["knowledge"].to_index_schema(_inspected_schema()) - loaded.validate_search( + loaded.indexes["knowledge"].validate_search( schema=schema, supports_native_hybrid_search=False, ) diff --git a/tests/unit/test_mcp/test_search_tool_unit.py b/tests/unit/test_mcp/test_search_tool_unit.py index d5a92244..aaeae595 100644 --- a/tests/unit/test_mcp/test_search_tool_unit.py +++ b/tests/unit/test_mcp/test_search_tool_unit.py @@ -5,6 +5,7 @@ from redisvl.mcp.config import MCPConfig from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError +from redisvl.mcp.runtime import BindingRuntime from redisvl.mcp.tools.search import ( _build_fallback_hybrid_kwargs, _build_search_tool_description, @@ -111,15 +112,18 @@ def __init__( self.registered_tools = [] self.native_hybrid_supported = False - async def get_index(self): - return self.index - - async def get_vectorizer(self): - if self.vectorizer is None: - raise RuntimeError("MCP server vectorizer is not configured") - return self.vectorizer + def resolve_binding(self, index_id=None): + return BindingRuntime( + binding_id="knowledge", + binding=self.config.indexes["knowledge"], + index=self.index, + schema=self.index.schema, + vectorizer=self.vectorizer, + supports_native_hybrid_search=self.native_hybrid_supported, + effective_read_only=False, + ) - async def run_guarded(self, operation_name, awaitable): + async def run_guarded(self, operation_name, awaitable, *, timeout_seconds=None): return await awaitable async def supports_native_hybrid_search(self): @@ -652,7 +656,7 @@ async def test_validate_search_rejects_reserved_score_metadata_field_names( ) with pytest.raises(ValueError, match="MCP-reserved score metadata names"): - config.validate_search( + config.indexes["knowledge"].validate_search( schema=schema, supports_native_hybrid_search=supports_native, ) @@ -701,7 +705,7 @@ async def test_search_records_rejects_native_only_hybrid_runtime_params(monkeypa ) with pytest.raises(ValueError, match="native hybrid search support"): - server.config.validate_search( + server.config.indexes["knowledge"].validate_search( schema=_schema(), supports_native_hybrid_search=False, ) diff --git a/tests/unit/test_mcp/test_server.py b/tests/unit/test_mcp/test_server.py index e88b8a38..cd949bf6 100644 --- a/tests/unit/test_mcp/test_server.py +++ b/tests/unit/test_mcp/test_server.py @@ -37,12 +37,18 @@ def _startup_schema() -> IndexSchema: ) -def _startup_config(): +def _binding_namespace( + *, requires_startup_vectorizer: bool = True, max_concurrency: int = 1 +): return SimpleNamespace( - runtime=SimpleNamespace(max_concurrency=1, startup_timeout_seconds=1), - server=SimpleNamespace(redis_url="redis://localhost:6379"), redis_name="idx", - requires_startup_vectorizer=True, + read_only=False, + requires_startup_vectorizer=requires_startup_vectorizer, + runtime=SimpleNamespace( + max_concurrency=max_concurrency, + startup_timeout_seconds=1, + request_timeout_seconds=1, + ), vectorizer=SimpleNamespace( class_name="FakeVectorizer", to_init_kwargs=lambda: {}, @@ -51,6 +57,22 @@ def _startup_config(): ) +def _startup_config(indexes=None): + return SimpleNamespace( + server=SimpleNamespace(redis_url="redis://localhost:6379"), + indexes=indexes or {"knowledge": _binding_namespace()}, + ) + + +def _patch_probe(monkeypatch, value: bool = False): + async def fake_probe(index): + return value + + monkeypatch.setattr( + RedisVLMCPServer, "_probe_native_hybrid_search", staticmethod(fake_probe) + ) + + @pytest.mark.asyncio async def test_server_registers_fastmcp_lifespan(monkeypatch): captured = {} @@ -97,15 +119,12 @@ async def test_run_guarded_rejects_before_startup(): future.set_result(None) with pytest.raises(RuntimeError, match="not running"): - await server.run_guarded("test", future) + await server.run_guarded("test", future, timeout_seconds=1) @pytest.mark.asyncio async def test_run_guarded_rejects_after_shutdown(): server = RedisVLMCPServer(_dummy_settings()) - server.config = SimpleNamespace( - runtime=SimpleNamespace(request_timeout_seconds=1, max_concurrency=1) - ) server._semaphore = asyncio.Semaphore(1) await server.shutdown() @@ -113,7 +132,66 @@ async def test_run_guarded_rejects_after_shutdown(): future = asyncio.get_running_loop().create_future() future.set_result(None) with pytest.raises(RuntimeError, match="not running"): - await server.run_guarded("test", future) + await server.run_guarded("test", future, timeout_seconds=1) + + +@pytest.mark.asyncio +async def test_run_guarded_uses_per_binding_timeout(monkeypatch): + server = RedisVLMCPServer(_dummy_settings()) + server._semaphore = asyncio.Semaphore(1) + server.config = _startup_config() + server._lifecycle_state = server._LifecycleState.RUNNING + + captured = {} + + async def fake_wait_for(awaitable, timeout): + captured["timeout"] = timeout + return await awaitable + + monkeypatch.setattr("redisvl.mcp.server.asyncio.wait_for", fake_wait_for) + + future = asyncio.get_running_loop().create_future() + future.set_result("ok") + + result = await server.run_guarded("test", future, timeout_seconds=42) + + assert result == "ok" + assert captured["timeout"] == 42 + + +@pytest.mark.asyncio +async def test_startup_sizes_semaphore_from_max_binding_concurrency(monkeypatch): + monkeypatch.setattr( + "redisvl.mcp.server.FastMCP.__init__", lambda self, *a, **k: None + ) + config = _startup_config( + indexes={ + "knowledge": _binding_namespace(max_concurrency=4), + "tickets": _binding_namespace(max_concurrency=9), + } + ) + monkeypatch.setattr("redisvl.mcp.server.load_mcp_config", lambda path: config) + + captured = {} + + def fake_semaphore(value): + captured["value"] = value + return SimpleNamespace(value=value) + + monkeypatch.setattr("redisvl.mcp.server.asyncio.Semaphore", fake_semaphore) + + async def fake_initialize_binding(self, binding_id, binding): + return SimpleNamespace(binding_id=binding_id) + + monkeypatch.setattr( + RedisVLMCPServer, "_initialize_binding", fake_initialize_binding + ) + monkeypatch.setattr(RedisVLMCPServer, "_register_tools", lambda self: None) + + server = RedisVLMCPServer(_dummy_settings()) + await server._initialize_runtime_resources() + + assert captured["value"] == 9 @pytest.mark.asyncio @@ -123,11 +201,7 @@ async def test_startup_failure_leaves_server_stopped(monkeypatch): ) monkeypatch.setattr( "redisvl.mcp.server.load_mcp_config", - lambda path: SimpleNamespace( - runtime=SimpleNamespace(max_concurrency=1, startup_timeout_seconds=1), - server=SimpleNamespace(redis_url="redis://localhost:6379"), - redis_name="idx", - ), + lambda path: _startup_config(), ) async def fail_connection(**kwargs): @@ -146,6 +220,46 @@ async def fail_connection(**kwargs): assert server._lifecycle_state.name == "STOPPED" assert server.config is None assert server._semaphore is None + assert server._bindings == {} + + +@pytest.mark.asyncio +async def test_startup_tears_down_when_a_post_init_step_fails(monkeypatch): + """Initialization succeeds but a later step raises; resources must be freed.""" + monkeypatch.setattr( + "redisvl.mcp.server.FastMCP.__init__", lambda self, *a, **k: None + ) + + async def fake_initialize(self): + # Simulate a fully initialized runtime (a live binding) ... + self.config = SimpleNamespace() + self._semaphore = SimpleNamespace() + self._bindings = { + "knowledge": SimpleNamespace( + binding_id="knowledge", index=None, vectorizer=None + ) + } + + async def fail_mark_running(self): + # ... then fail on the post-init step. + raise RuntimeError("mark running failed") + + monkeypatch.setattr( + RedisVLMCPServer, "_initialize_runtime_resources", fake_initialize + ) + monkeypatch.setattr(RedisVLMCPServer, "_mark_running", fail_mark_running) + + server = RedisVLMCPServer(_dummy_settings()) + + with pytest.raises(RuntimeError, match="mark running failed"): + await server.startup() + + # Teardown ran from startup()'s handler even though the failure was after + # initialization, so nothing leaks and the server is left stopped. + assert server._lifecycle_state.name == "STOPPED" + assert server._bindings == {} + assert server.config is None + assert server._semaphore is None @pytest.mark.asyncio @@ -170,7 +284,7 @@ async def aclose(self): async def fake_connect(self, timeout): return client - async def fail_load_schema(self, client, timeout): + async def fail_load_schema(self, binding, client, timeout): raise RuntimeError("schema load failed") monkeypatch.setattr(RedisVLMCPServer, "_connect_redis_client", fake_connect) @@ -185,7 +299,7 @@ async def fail_load_schema(self, client, timeout): assert server._lifecycle_state.name == "STOPPED" assert server.config is None assert server._semaphore is None - assert server._index is None + assert server._bindings == {} @pytest.mark.asyncio @@ -213,13 +327,10 @@ async def aclose(self): async def fake_connect(self, timeout): return client - async def fake_load_schema(self, client, timeout): + async def fake_load_schema(self, binding, client, timeout): return _startup_schema() - async def fake_supports_native_hybrid_search(self): - return False - - async def fail_vectorizer(self, schema, timeout): + async def fail_vectorizer(self, binding, schema, timeout): raise RuntimeError("vectorizer init failed") async def fake_disconnect(self): @@ -227,11 +338,7 @@ async def fake_disconnect(self): monkeypatch.setattr(RedisVLMCPServer, "_connect_redis_client", fake_connect) monkeypatch.setattr(RedisVLMCPServer, "_load_effective_schema", fake_load_schema) - monkeypatch.setattr( - RedisVLMCPServer, - "supports_native_hybrid_search", - fake_supports_native_hybrid_search, - ) + _patch_probe(monkeypatch, value=False) monkeypatch.setattr(RedisVLMCPServer, "_initialize_vectorizer", fail_vectorizer) monkeypatch.setattr( "redisvl.mcp.server.AsyncSearchIndex.disconnect", @@ -249,7 +356,7 @@ async def fake_disconnect(self): assert server._lifecycle_state.name == "STOPPED" assert server.config is None assert server._semaphore is None - assert server._index is None + assert server._bindings == {} @pytest.mark.asyncio @@ -269,7 +376,7 @@ async def aclose(self): async def fake_connect(self, timeout): return FakeClient() - async def fake_load_schema(self, client, timeout): + async def fake_load_schema(self, binding, client, timeout): return IndexSchema.from_dict( { "index": { @@ -295,11 +402,8 @@ async def fake_load_schema(self, client, timeout): } ) - async def fake_supports_native_hybrid_search(self): - return False - - async def fake_initialize_vectorizer(self, schema, timeout): - self._vectorizer = SimpleNamespace(dims=3) + async def fake_initialize_vectorizer(self, binding, schema, timeout): + return SimpleNamespace(dims=3) registered_schemas = [] @@ -311,11 +415,7 @@ async def fake_disconnect(self): monkeypatch.setattr(RedisVLMCPServer, "_connect_redis_client", fake_connect) monkeypatch.setattr(RedisVLMCPServer, "_load_effective_schema", fake_load_schema) - monkeypatch.setattr( - RedisVLMCPServer, - "supports_native_hybrid_search", - fake_supports_native_hybrid_search, - ) + _patch_probe(monkeypatch, value=False) monkeypatch.setattr( RedisVLMCPServer, "_initialize_vectorizer", fake_initialize_vectorizer ) diff --git a/tests/unit/test_mcp/test_server_unit.py b/tests/unit/test_mcp/test_server_unit.py index 13ba23d5..2aa73473 100644 --- a/tests/unit/test_mcp/test_server_unit.py +++ b/tests/unit/test_mcp/test_server_unit.py @@ -2,6 +2,8 @@ import pytest +from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError +from redisvl.mcp.runtime import BindingRuntime from redisvl.mcp.server import RedisVLMCPServer @@ -29,14 +31,112 @@ async def _get_client(self): @pytest.mark.asyncio -async def test_supports_native_hybrid_search_caches_runtime_probe(monkeypatch): +async def test_probe_native_hybrid_search_detects_support(monkeypatch): client = FakeClient() - server = RedisVLMCPServer.__new__(RedisVLMCPServer) - server._index = FakeIndex(client) - server._supports_native_hybrid_search = None + index = FakeIndex(client) monkeypatch.setattr("redisvl.mcp.server.redis_py_version", "7.1.0") - assert await server.supports_native_hybrid_search() is True - assert await server.supports_native_hybrid_search() is True + assert await RedisVLMCPServer._probe_native_hybrid_search(index) is True assert client.info_calls == 1 + + +@pytest.mark.asyncio +async def test_probe_native_hybrid_search_false_for_old_redis_py(monkeypatch): + client = FakeClient() + index = FakeIndex(client) + + monkeypatch.setattr("redisvl.mcp.server.redis_py_version", "7.0.0") + + assert await RedisVLMCPServer._probe_native_hybrid_search(index) is False + # Old redis-py short-circuits before querying the server. + assert client.info_calls == 0 + + +def _binding_runtime(binding_id: str) -> BindingRuntime: + return BindingRuntime( + binding_id=binding_id, + binding=SimpleNamespace(), + index=SimpleNamespace(), + schema=SimpleNamespace(), + vectorizer=None, + supports_native_hybrid_search=False, + effective_read_only=False, + ) + + +def _server_with_bindings(*binding_ids: str) -> RedisVLMCPServer: + server = RedisVLMCPServer.__new__(RedisVLMCPServer) + server._bindings = {bid: _binding_runtime(bid) for bid in binding_ids} + return server + + +def test_resolve_binding_before_startup_raises(): + server = RedisVLMCPServer.__new__(RedisVLMCPServer) + server._bindings = {} + + with pytest.raises(RuntimeError, match="not been started"): + server.resolve_binding(None) + + +def test_resolve_binding_defaults_to_sole_binding(): + server = _server_with_bindings("knowledge") + + assert server.resolve_binding(None).binding_id == "knowledge" + + +def test_resolve_binding_requires_index_when_multiple_configured(): + server = _server_with_bindings("knowledge", "tickets") + + with pytest.raises(RedisVLMCPError) as excinfo: + server.resolve_binding(None) + + assert excinfo.value.code == MCPErrorCode.INVALID_REQUEST + assert "knowledge" in str(excinfo.value) + assert "tickets" in str(excinfo.value) + + +def test_resolve_binding_routes_to_named_index(): + server = _server_with_bindings("knowledge", "tickets") + + assert server.resolve_binding("tickets").binding_id == "tickets" + + +def test_resolve_binding_rejects_unknown_index(): + server = _server_with_bindings("knowledge", "tickets") + + with pytest.raises(RedisVLMCPError) as excinfo: + server.resolve_binding("missing") + + assert excinfo.value.code == MCPErrorCode.INVALID_REQUEST + assert "missing" in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_teardown_continues_when_a_binding_fails_to_close(monkeypatch): + """A failed close on one binding must not leak the remaining bindings.""" + server = _server_with_bindings("knowledge", "tickets") + server.config = SimpleNamespace() + server._semaphore = SimpleNamespace() + server._tools_registered = True + + closed: list[str] = [] + + async def fake_close_resources(self, *, index, vectorizer): + # Fail on the first binding; the loop must still reach the second. + if not closed: + closed.append("knowledge") + raise RuntimeError("disconnect failed") + closed.append("tickets") + + monkeypatch.setattr(RedisVLMCPServer, "_close_resources", fake_close_resources) + + await server._teardown_runtime() + + # Both bindings were attempted despite the first one raising. + assert closed == ["knowledge", "tickets"] + # Binding state is cleared... + assert server._bindings == {} + # ...but tool registration is instance-level and must survive teardown, so a + # stop/start does not re-register the same tool names on the FastMCP object. + assert server._tools_registered is True diff --git a/tests/unit/test_mcp/test_upsert_tool_unit.py b/tests/unit/test_mcp/test_upsert_tool_unit.py index 15d3d5ca..5c2e059d 100644 --- a/tests/unit/test_mcp/test_upsert_tool_unit.py +++ b/tests/unit/test_mcp/test_upsert_tool_unit.py @@ -6,6 +6,7 @@ from redisvl.mcp.config import MCPConfig from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError +from redisvl.mcp.runtime import BindingRuntime from redisvl.mcp.tools.upsert import register_upsert_tool, upsert_records from redisvl.redis.utils import array_to_buffer from redisvl.schema import IndexSchema @@ -149,6 +150,7 @@ def __init__( max_upsert_records: int = 5, skip_embedding_if_present: bool = True, vectorizer: FakeVectorizer | None = None, + effective_read_only: bool = False, ): self.config = _config( storage_type, @@ -163,16 +165,22 @@ def __init__( self.index = FakeIndex(storage_type, include_vector_field=include_vector_field) self.vectorizer = vectorizer or FakeVectorizer() if include_vectorizer else None self.registered_tools = [] + self.effective_read_only = effective_read_only + + def resolve_binding(self, index_id=None): + return BindingRuntime( + binding_id="knowledge", + binding=self.config.indexes["knowledge"], + index=self.index, + schema=self.index.schema, + vectorizer=self.vectorizer, + supports_native_hybrid_search=False, + effective_read_only=self.effective_read_only, + ) - async def get_index(self): - return self.index - - async def get_vectorizer(self): - if self.vectorizer is None: - raise RuntimeError("MCP server vectorizer is not configured") - return self.vectorizer - - async def run_guarded(self, operation_name: str, awaitable: Any): + async def run_guarded( + self, operation_name: str, awaitable: Any, *, timeout_seconds=None + ): return await awaitable def tool(self, name=None, description=None, **kwargs): @@ -449,6 +457,18 @@ async def test_upsert_records_surfaces_partial_write_possible_on_backend_failure assert isinstance(exc_info.value.__cause__, RedisError) +@pytest.mark.asyncio +async def test_upsert_records_rejects_writes_to_read_only_binding(): + server = FakeServer(effective_read_only=True) + + with pytest.raises(RedisVLMCPError) as exc_info: + await upsert_records(server, records=[{"content": "alpha doc"}]) + + assert exc_info.value.code == MCPErrorCode.FORBIDDEN + # The write is rejected before any backend load is attempted. + assert server.index.load_calls == [] + + def test_register_upsert_tool_uses_default_and_override_descriptions(): default_server = FakeServer() register_upsert_tool(default_server) From 6bd9d309061e9af691c017da3e7f6dc872d9d8ce Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Wed, 1 Jul 2026 11:41:27 +0200 Subject: [PATCH 2/5] feat(mcp): add list-indexes discovery tool (RAAE-1605) (#630) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > Stacked on #629 (RAAE-1604). Review/merge that first; this PR targets the 1604 branch so the diff is scoped to the discovery tool. ## Motivation Once a single MCP server can expose multiple logical indexes ([RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)), clients need a lightweight way to discover what's available so they can pick the right index instead of guessing. This PR ([RAAE-1605](https://redislabs.atlassian.net/browse/RAAE-1605)) adds an always-registered, read-only `list-indexes` tool for exactly that, and it grounds discovery in the schema the server already inspected at startup rather than asking users to re-declare field metadata in config. ## Implementation The new tool (`redisvl/mcp/tools/list_indexes.py`) returns one entry per configured binding, in configured order: the logical `id`, an optional `description`, an `upsert_available` flag, the shared filterable `fields`, and β€” only when explicitly configured β€” a `limits` object. `upsert_available` is simply `not effective_read_only`, so it already reflects both the global `--read-only` flag and the per-index `read_only` policy resolved at startup. The `fields` list is built from the binding's effective (inspected + overridden) schema that already lives on its `BindingRuntime`, so the output stays consistent with what the index actually contains; the vector field and the configured default embed-source text field are omitted because they are implementation inputs rather than fields a client would filter on. The Redis index name (`redis_name`) is deliberately never exposed. Limits are included only when the operator set them explicitly β€” detected via the runtime model's `model_fields_set` β€” so defaults don't masquerade as deliberate overrides; per the contract this covers `max_limit` and `max_upsert_records`. The tool is registered unconditionally during the server's tool registration (alongside `search-records` and the conditionally-registered `upsert-records`) and is gated by the same read scope as search when auth is enabled, since it is read-only. - Output is deterministic and ordered by configured binding. - No new configuration surface or settings are required. ## Verification - `mypy` clean; `black`/`isort` formatted. - New unit coverage: field omission (vector + embed-source), description/limits inclusion rules, `redis_name` secrecy, read-only reflection, single- and multi-binding output, and tool registration. - New integration test starts a real two-binding server (one vector, one fulltext) and asserts the discovered fields come from the inspected schema and follow the omission rules. - Full MCP suite green: 178 unit + 45 integration (2 skipped on Redis-version gates) against Redis 8. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1605]: https://redislabs.atlassian.net/browse/RAAE-1605?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > **Low Risk** > Read-only discovery metadata with no new config; behavior is additive and gated like existing search tools when auth is enabled. > > **Overview** > Adds a read-only **`list-indexes`** MCP tool so clients can discover logical indexes on multi-binding servers before calling **`search-records`** or **`upsert-records`**. > > The tool is **always registered** during `_register_tools` (alongside search; upsert remains conditional). Each binding is returned in config order with **`id`**, optional **`description`**, **`upsert_available`** (`not effective_read_only`), filterable **`fields`** from the startup-inspected schema (vector and default embed-source text omitted), and **`limits`** only when **`max_limit`** / **`max_upsert_records`** were explicitly set. **`redis_name`** is never exposed; empty bindings raise **`RuntimeError`** like other pre-startup paths. When auth is on, the tool uses the same **read scope** as search. > > Unit and integration tests cover payload rules, registration, and a two-binding startup scenario. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 45ce686e0ef11a4872cab82836aed3eccf5be659. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). --------- Co-authored-by: Claude Opus 4.8 (1M context) --- redisvl/mcp/server.py | 3 + redisvl/mcp/tools/list_indexes.py | 96 ++++++++ .../test_mcp/test_server_startup.py | 76 ++++++ .../test_mcp/test_list_indexes_tool_unit.py | 231 ++++++++++++++++++ tests/unit/test_mcp/test_server.py | 3 + 5 files changed, 409 insertions(+) create mode 100644 redisvl/mcp/tools/list_indexes.py create mode 100644 tests/unit/test_mcp/test_list_indexes_tool_unit.py diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index b8955c48..899bb6f7 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -15,6 +15,7 @@ from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError from redisvl.mcp.runtime import BindingRuntime from redisvl.mcp.settings import MCPSettings +from redisvl.mcp.tools.list_indexes import register_list_indexes_tool from redisvl.mcp.tools.search import register_search_tool from redisvl.mcp.tools.upsert import register_upsert_tool from redisvl.redis.connection import RedisConnectionFactory, is_version_gte @@ -246,6 +247,8 @@ def _register_tools(self) -> None: if len(self._bindings) == 1: search_schema = next(iter(self._bindings.values())).schema + # Discovery is always available so clients can enumerate indexes. + register_list_indexes_tool(self) register_search_tool(self, search_schema) if not self.mcp_settings.read_only: register_upsert_tool(self) diff --git a/redisvl/mcp/tools/list_indexes.py b/redisvl/mcp/tools/list_indexes.py new file mode 100644 index 00000000..58a6adef --- /dev/null +++ b/redisvl/mcp/tools/list_indexes.py @@ -0,0 +1,96 @@ +from typing import TYPE_CHECKING, Any + +from redisvl.mcp.auth import ensure_tool_scope +from redisvl.mcp.runtime import BindingRuntime + +if TYPE_CHECKING: + from redisvl.mcp.server import RedisVLMCPServer + +DEFAULT_LIST_INDEXES_DESCRIPTION = ( + "List the logical indexes configured on this server. Each entry reports the " + "index id, an optional description, whether upsert is available, the " + "filterable fields discovered from the index, and any explicitly configured " + "limits. Call this first on a multi-index server to choose the correct " + "index for search-records or upsert-records." +) + +# Runtime limits surfaced to clients, included only when explicitly configured. +_LIMIT_FIELDS = ("max_limit", "max_upsert_records") + + +def _binding_fields(binding_runtime: BindingRuntime) -> list[dict[str, str]]: + """Return a binding's shared filterable fields from its inspected schema. + + The vector field and the configured default embed-source text field are + omitted: they are implementation inputs, not fields a client filters on. + """ + embed_source = binding_runtime.binding.runtime.default_embed_text_field + fields: list[dict[str, str]] = [] + for field in binding_runtime.schema.fields.values(): + field_type = str(getattr(field.type, "value", field.type)) + if field_type.lower() == "vector": + continue + if field.name == embed_source: + continue + fields.append({"name": field.name, "type": field_type}) + return fields + + +def _binding_limits(binding_runtime: BindingRuntime) -> dict[str, int]: + """Return runtime limits that were explicitly configured for the binding. + + Defaults are intentionally excluded so the output reflects deliberate + overrides rather than implementation defaults. + """ + runtime = binding_runtime.binding.runtime + configured = runtime.model_fields_set + return { + name: getattr(runtime, name) for name in _LIMIT_FIELDS if name in configured + } + + +def _describe_binding(binding_runtime: BindingRuntime) -> dict[str, Any]: + """Build the deterministic discovery payload for a single binding.""" + entry: dict[str, Any] = {"id": binding_runtime.binding_id} + if binding_runtime.binding.description is not None: + entry["description"] = binding_runtime.binding.description + # Reflects both global read-only and the per-index read_only policy. + entry["upsert_available"] = not binding_runtime.effective_read_only + entry["fields"] = _binding_fields(binding_runtime) + limits = _binding_limits(binding_runtime) + if limits: + entry["limits"] = limits + return entry + + +def list_indexes(server: "RedisVLMCPServer") -> dict[str, Any]: + """Return the discovery payload for every configured binding. + + The Redis index name (``redis_name``) is intentionally never exposed. + """ + # Mirror resolve_binding: with no bindings the server is not started (or has + # been torn down), so fail loudly rather than return an empty list that a + # client could misread as "no indexes configured". + if not server._bindings: + raise RuntimeError("MCP server has not been started") + return { + "indexes": [ + _describe_binding(binding_runtime) + for binding_runtime in server._bindings.values() + ], + } + + +def register_list_indexes_tool(server: "RedisVLMCPServer") -> None: + """Register the always-available, read-only `list-indexes` MCP tool.""" + + async def list_indexes_tool(): + """FastMCP wrapper for the `list-indexes` tool.""" + auth_config = getattr(server, "auth_config", None) + read_scope = auth_config.read_scope if auth_config is not None else None + ensure_tool_scope(server, read_scope) + return list_indexes(server) + + server.tool(name="list-indexes", description=DEFAULT_LIST_INDEXES_DESCRIPTION)( + list_indexes_tool + ) diff --git a/tests/integration/test_mcp/test_server_startup.py b/tests/integration/test_mcp/test_server_startup.py index c278e57e..ec7de37e 100644 --- a/tests/integration/test_mcp/test_server_startup.py +++ b/tests/integration/test_mcp/test_server_startup.py @@ -9,6 +9,7 @@ from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError from redisvl.mcp.server import RedisVLMCPServer from redisvl.mcp.settings import MCPSettings +from redisvl.mcp.tools.list_indexes import list_indexes from redisvl.redis.connection import is_version_gte from redisvl.schema import IndexSchema from tests.conftest import ( @@ -730,3 +731,78 @@ async def test_server_startup_fails_when_one_binding_is_invalid( assert server._lifecycle_state.name == "STOPPED" assert server._bindings == {} + + +@pytest.mark.asyncio +async def test_list_indexes_derives_fields_from_inspected_schema( + monkeypatch, existing_index, multi_index_config_path +): + knowledge = await existing_index(index_name="mcp-list-knowledge") + tickets = await existing_index(index_name="mcp-list-tickets") + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + server = RedisVLMCPServer( + MCPSettings( + config=multi_index_config_path( + { + # Vector binding: content is the embed source. + "knowledge": { + "redis_name": knowledge.name, + "description": "Product docs", + "vectorizer": { + "class": "FakeVectorizer", + "model": "fake-model", + "dims": 3, + }, + "search": {"type": "vector"}, + "runtime": { + "text_field_name": "content", + "vector_field_name": "embedding", + "default_embed_text_field": "content", + "max_limit": 25, + }, + }, + # Fulltext binding: no embed source, read-only. + "tickets": { + "redis_name": tickets.name, + "read_only": True, + "search": {"type": "fulltext"}, + "runtime": {"text_field_name": "content"}, + }, + } + ) + ) + ) + + await server.startup() + + try: + result = list_indexes(server) + indexes = {entry["id"]: entry for entry in result["indexes"]} + + # Both bindings are discoverable; redis_name is never leaked. + assert set(indexes) == {"knowledge", "tickets"} + for entry in indexes.values(): + assert "redis_name" not in entry + assert knowledge.name not in entry.values() + assert tickets.name not in entry.values() + + # Fields come from the inspected schema. The vector field is always + # omitted; the embed-source field is omitted only where configured. + knowledge_fields = {f["name"] for f in indexes["knowledge"]["fields"]} + tickets_fields = {f["name"] for f in indexes["tickets"]["fields"]} + assert "embedding" not in knowledge_fields + assert "embedding" not in tickets_fields + assert "content" not in knowledge_fields # embed source omitted + assert "content" in tickets_fields # no embed source configured + + # Per-index write policy and explicit limits are reflected. + assert indexes["knowledge"]["upsert_available"] is True + assert indexes["tickets"]["upsert_available"] is False + assert indexes["knowledge"]["limits"] == {"max_limit": 25} + assert "limits" not in indexes["tickets"] + assert indexes["knowledge"]["description"] == "Product docs" + finally: + await server.shutdown() diff --git a/tests/unit/test_mcp/test_list_indexes_tool_unit.py b/tests/unit/test_mcp/test_list_indexes_tool_unit.py new file mode 100644 index 00000000..0d56b083 --- /dev/null +++ b/tests/unit/test_mcp/test_list_indexes_tool_unit.py @@ -0,0 +1,231 @@ +from types import SimpleNamespace +from typing import Any + +import pytest + +from redisvl.mcp.config import MCPConfig +from redisvl.mcp.runtime import BindingRuntime +from redisvl.mcp.tools.list_indexes import list_indexes, register_list_indexes_tool +from redisvl.schema import IndexSchema + + +def _schema() -> IndexSchema: + return IndexSchema.from_dict( + { + "index": { + "name": "docs-index", + "prefix": "doc", + "storage_type": "hash", + }, + "fields": [ + {"name": "title", "type": "text"}, + {"name": "content", "type": "text"}, + {"name": "category", "type": "tag"}, + {"name": "rating", "type": "numeric"}, + { + "name": "embedding", + "type": "vector", + "attrs": { + "algorithm": "flat", + "dims": 3, + "distance_metric": "cosine", + "datatype": "float32", + }, + }, + ], + } + ) + + +def _binding_runtime( + binding_id: str = "knowledge", + *, + runtime: dict[str, Any] | None = None, + description: str | None = None, + read_only: bool = False, + effective_read_only: bool = False, + schema: IndexSchema | None = None, +) -> BindingRuntime: + runtime_config = { + "vector_field_name": "embedding", + "default_embed_text_field": "content", + } + if runtime: + runtime_config.update(runtime) + + binding_dict: dict[str, Any] = { + "redis_name": f"{binding_id}-redis-name", + "read_only": read_only, + "vectorizer": {"class": "FakeVectorizer", "model": "test-model"}, + "search": {"type": "vector"}, + "runtime": runtime_config, + } + if description is not None: + binding_dict["description"] = description + + config = MCPConfig.model_validate( + { + "server": {"redis_url": "redis://localhost:6379"}, + "indexes": {binding_id: binding_dict}, + } + ) + return BindingRuntime( + binding_id=binding_id, + binding=config.indexes[binding_id], + index=SimpleNamespace(), + schema=schema or _schema(), + vectorizer=None, + supports_native_hybrid_search=False, + effective_read_only=effective_read_only, + ) + + +class FakeServer: + def __init__(self, bindings: list[BindingRuntime]): + self._bindings = {rt.binding_id: rt for rt in bindings} + self.mcp_settings = SimpleNamespace() + self.auth_config = None + self._auth_enabled = False + self.registered_tools: list[dict[str, Any]] = [] + + def tool(self, name=None, description=None, **kwargs): + def decorator(fn): + self.registered_tools.append( + {"name": name, "description": description, "fn": fn} + ) + return fn + + return decorator + + +def test_list_indexes_raises_when_no_bindings(): + # Before startup / after shutdown _bindings is empty; discovery must fail + # loudly rather than return an empty list a client could misread. + server = FakeServer([]) + + with pytest.raises(RuntimeError, match="has not been started"): + list_indexes(server) + + +def test_list_indexes_minimal_single_binding(): + server = FakeServer([_binding_runtime()]) + + result = list_indexes(server) + + assert result == { + "indexes": [ + { + "id": "knowledge", + "upsert_available": True, + "fields": [ + {"name": "title", "type": "text"}, + {"name": "category", "type": "tag"}, + {"name": "rating", "type": "numeric"}, + ], + } + ] + } + + +def test_list_indexes_omits_vector_and_embed_source_fields(): + server = FakeServer([_binding_runtime()]) + + fields = list_indexes(server)["indexes"][0]["fields"] + field_names = [field["name"] for field in fields] + + # embedding is the vector field; content is the default embed-source field. + assert "embedding" not in field_names + assert "content" not in field_names + + +def test_list_indexes_includes_description_when_configured(): + server = FakeServer([_binding_runtime(description="Product docs and runbooks")]) + + entry = list_indexes(server)["indexes"][0] + + assert entry["description"] == "Product docs and runbooks" + + +def test_list_indexes_omits_description_when_absent(): + server = FakeServer([_binding_runtime()]) + + assert "description" not in list_indexes(server)["indexes"][0] + + +def test_list_indexes_upsert_available_reflects_effective_read_only(): + server = FakeServer( + [ + _binding_runtime("knowledge", effective_read_only=False), + _binding_runtime("tickets", read_only=True, effective_read_only=True), + ] + ) + + indexes = {entry["id"]: entry for entry in list_indexes(server)["indexes"]} + + assert indexes["knowledge"]["upsert_available"] is True + assert indexes["tickets"]["upsert_available"] is False + + +def test_list_indexes_includes_limits_only_when_explicitly_configured(): + server = FakeServer( + [ + _binding_runtime( + "explicit", + runtime={"max_limit": 25, "max_upsert_records": 64}, + ), + _binding_runtime("defaults"), + ] + ) + + indexes = {entry["id"]: entry for entry in list_indexes(server)["indexes"]} + + assert indexes["explicit"]["limits"] == { + "max_limit": 25, + "max_upsert_records": 64, + } + assert "limits" not in indexes["defaults"] + + +def test_list_indexes_includes_only_the_explicitly_set_limit(): + server = FakeServer([_binding_runtime(runtime={"max_limit": 25})]) + + entry = list_indexes(server)["indexes"][0] + + assert entry["limits"] == {"max_limit": 25} + + +def test_list_indexes_never_exposes_redis_name(): + server = FakeServer([_binding_runtime()]) + + entry = list_indexes(server)["indexes"][0] + + assert "redis_name" not in entry + assert "knowledge-redis-name" not in entry.values() + + +def test_list_indexes_preserves_binding_order(): + server = FakeServer( + [ + _binding_runtime("knowledge"), + _binding_runtime("tickets"), + ] + ) + + ids = [entry["id"] for entry in list_indexes(server)["indexes"]] + + assert ids == ["knowledge", "tickets"] + + +@pytest.mark.asyncio +async def test_register_list_indexes_tool_is_read_only_and_callable(): + server = FakeServer([_binding_runtime()]) + + register_list_indexes_tool(server) + + assert len(server.registered_tools) == 1 + tool = server.registered_tools[0] + assert tool["name"] == "list-indexes" + assert tool["description"] + + result = await tool["fn"]() + assert result == list_indexes(server) diff --git a/tests/unit/test_mcp/test_server.py b/tests/unit/test_mcp/test_server.py index cd949bf6..9179bdd5 100644 --- a/tests/unit/test_mcp/test_server.py +++ b/tests/unit/test_mcp/test_server.py @@ -423,6 +423,9 @@ async def fake_disconnect(self): "redisvl.mcp.server.register_search_tool", fake_register_search_tool ) monkeypatch.setattr("redisvl.mcp.server.register_upsert_tool", lambda server: None) + monkeypatch.setattr( + "redisvl.mcp.server.register_list_indexes_tool", lambda server: None + ) monkeypatch.setattr( "redisvl.mcp.server.AsyncSearchIndex.disconnect", fake_disconnect, From 043e486cc8988772f847125314e9f758465ae7e3 Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Thu, 2 Jul 2026 13:38:56 +0200 Subject: [PATCH 3/5] feat(mcp): add index routing to search-records (RAAE-1606) (#631) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation With a single MCP server now able to expose multiple logical index bindings ([RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)) and advertise them via `list-indexes` ([RAAE-1605](https://redislabs.atlassian.net/browse/RAAE-1605)), the `search-records` tool still implicitly resolved the sole binding. On a multi-binding server it had no way to say *which* index a query should run against. This change ([RAAE-1606](https://redislabs.atlassian.net/browse/RAAE-1606)) makes that routing explicit while keeping the public contract backwards-safe for existing single-index callers. The design goal is that v1 clients see no behavioral change: when exactly one binding is configured and the caller omits `index`, the request resolves to that binding exactly as before. Routing only becomes mandatory once ambiguity exists. Query construction, validation, pagination, filtering, and the configured search mode all remain owned by the selected binding β€” this ticket adds only the selection and a confirmation echo, not new query behavior. ## Implemented changes `search-records` gains an optional `index` argument naming the logical binding to query. Resolution flows through the shared `resolve_binding` routing introduced in RAAE-1604, so the three cases fall out consistently: an omitted `index` with one binding resolves to that binding; an omitted `index` with multiple bindings returns `invalid_request`; and an unknown id returns `invalid_request`. The resolved logical id is echoed back as the `index` field of the response payload so multi-index clients can confirm where a query actually ran. All downstream work (limits, schema, vectorizer, search config) continues to read from the resolved binding's runtime. When multiple bindings are configured the tool description is ambiguous about fields, so instead of emitting per-field filter hints it now appends a short routing note directing the client to call `list-indexes` first and pass the chosen id as `index`. Single-binding servers keep their full schema-derived description unchanged. Minor additional changes: - The FastMCP wrapper exposes `index` as a tool parameter so MCP clients can supply it. - Unit coverage for default-to-sole-binding, named routing, unknown-id rejection, the wrapper param, and the ambiguous-schema description note. - Integration coverage for a two-binding server: routing to each named binding, omitted-index rejection, unknown-id rejection, and single-binding echo. ## Verification - `make format` (isort + black) and mypy clean on changed files. - Full MCP suite: **232 passed, 2 skipped** (Redis-version-gated) across unit + integration. ## Stacking This PR targets `feature/raae-1605-list-indexes` so its diff stays scoped to search routing. Review/merge bottom-up: [#629](https://github.com/redis/redis-vl-python/pull/629) β†’ [#630](https://github.com/redis/redis-vl-python/pull/630) β†’ this PR. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1605]: https://redislabs.atlassian.net/browse/RAAE-1605?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1606]: https://redislabs.atlassian.net/browse/RAAE-1606?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > **Low Risk** > Backward-compatible for single-index callers; changes are limited to MCP search routing and an additive response field, with validation delegated to existing `resolve_binding`. > > **Overview** > The MCP **`search-records`** tool now accepts an optional **`index`** argument so clients can target a specific logical binding on multi-index servers. Resolution goes through **`resolve_binding`**: omitting **`index`** still works when only one binding exists; with multiple bindings, **`index`** is required and unknown ids return **`invalid_request`**. > > Successful responses include a new **`index`** field with the resolved binding id so callers can confirm routing. The FastMCP wrapper exposes **`index`** as a tool parameter. > > When the server has multiple indexes, the tool description skips per-schema filter/return-field hints and instead tells clients to call **`list-indexes`** and pass the chosen id as **`index`**. Single-index servers keep the full schema-derived description. > > Unit and integration tests cover named routing, omitted-index errors on multi-binding setups, unknown-index rejection, single-binding echo behavior, and the ambiguous-schema description. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c2f30d5f29c10ca25ea451ee8a3fdd581cc3b70f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). --------- Co-authored-by: Claude Opus 4.8 (1M context) --- redisvl/mcp/tools/search.py | 23 +++- .../integration/test_mcp/test_search_tool.py | 102 ++++++++++++++++++ tests/unit/test_mcp/test_search_tool_unit.py | 67 ++++++++++++ 3 files changed, 187 insertions(+), 5 deletions(-) diff --git a/redisvl/mcp/tools/search.py b/redisvl/mcp/tools/search.py index 36d136f7..adbeab75 100644 --- a/redisvl/mcp/tools/search.py +++ b/redisvl/mcp/tools/search.py @@ -56,12 +56,15 @@ def _build_search_tool_description( """Build the `search-records` description from static text plus schema hints. With multiple bindings configured the schema is ambiguous (the caller picks - an index per call via `list-indexes`), so `schema` is None and only the - base description is returned. + an index per call via `list-indexes`), so per-field hints are omitted and a + routing note is appended instead. """ description = (base_description or DEFAULT_SEARCH_DESCRIPTION).strip() if schema is None: - return description + return ( + description + " Multiple indexes are configured: call list-indexes " + "first, then pass the chosen index id as the `index` argument." + ) # `exists` is currently accepted for any schema field in the MCP object filter. exists_fields = [field.name for field in schema.fields.values()] @@ -427,14 +430,21 @@ async def search_records( server: Any, *, query: str, + index: str | None = None, limit: int | None = None, offset: int = 0, filter: str | dict[str, Any] | None = None, return_fields: list[str] | None = None, ) -> dict[str, Any]: - """Execute `search-records` against the selected Redis index binding.""" + """Execute `search-records` against the selected Redis index binding. + + ``index`` names the logical binding to query. It is optional when exactly + one binding is configured (preserving single-index behavior) and required + when multiple bindings exist. The resolved logical id is echoed back in the + response so multi-index clients can confirm routing. + """ try: - rt = server.resolve_binding(None) + rt = server.resolve_binding(index) effective_limit, effective_return_fields = _validate_request( query=query, limit=limit, @@ -458,6 +468,7 @@ async def search_records( ) sliced_results = raw_results[offset : offset + effective_limit] return { + "index": rt.binding_id, "search_type": search_type, "offset": offset, "limit": effective_limit, @@ -485,6 +496,7 @@ def register_search_tool(server: Any, schema: IndexSchema | None) -> None: async def search_records_tool( query: str, + index: str | None = None, limit: int | None = None, offset: int = 0, filter: str | dict[str, Any] | None = None, @@ -497,6 +509,7 @@ async def search_records_tool( return await search_records( server, query=query, + index=index, limit=limit, offset=offset, filter=filter, diff --git a/tests/integration/test_mcp/test_search_tool.py b/tests/integration/test_mcp/test_search_tool.py index a59f11c9..03824fe6 100644 --- a/tests/integration/test_mcp/test_search_tool.py +++ b/tests/integration/test_mcp/test_search_tool.py @@ -214,6 +214,108 @@ async def started(search: dict, **kwargs) -> RedisVLMCPServer: await server.shutdown() +@pytest.fixture +async def multi_index_server( + monkeypatch, searchable_index, fulltext_only_index, tmp_path, redis_url +): + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + + config = { + "server": {"redis_url": redis_url}, + "indexes": { + "knowledge": { + "redis_name": searchable_index.schema.index.name, + "search": {"type": "vector"}, + "vectorizer": { + "class": "FakeVectorizer", + "model": "fake-model", + "dims": 3, + }, + "runtime": { + "text_field_name": "content", + "vector_field_name": "embedding", + "default_embed_text_field": "content", + "default_limit": 2, + "max_limit": 5, + }, + }, + "tickets": { + "redis_name": fulltext_only_index.schema.index.name, + "search": {"type": "fulltext", "params": {"stopwords": None}}, + "runtime": { + "text_field_name": "content", + "vector_field_name": None, + "default_embed_text_field": None, + "default_limit": 2, + "max_limit": 5, + }, + }, + }, + } + config_path = tmp_path / "multi-index-search.yaml" + config_path.write_text(yaml.safe_dump(config), encoding="utf-8") + + server = RedisVLMCPServer(MCPSettings(config=str(config_path))) + await server.startup() + try: + yield server + finally: + await server.shutdown() + + +@pytest.mark.asyncio +async def test_search_records_routes_to_named_binding(multi_index_server): + knowledge = await search_records( + multi_index_server, + query="science", + index="knowledge", + return_fields=["content", "category"], + ) + assert knowledge["index"] == "knowledge" + assert knowledge["search_type"] == "vector" + assert knowledge["results"] + + tickets = await search_records( + multi_index_server, + query="science", + index="tickets", + return_fields=["content", "category"], + ) + assert tickets["index"] == "tickets" + assert tickets["search_type"] == "fulltext" + assert tickets["results"] + + +@pytest.mark.asyncio +async def test_search_records_requires_index_when_multiple_bindings(multi_index_server): + with pytest.raises(RedisVLMCPError) as exc_info: + await search_records(multi_index_server, query="science") + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + + +@pytest.mark.asyncio +async def test_search_records_rejects_unknown_index_on_multi_binding( + multi_index_server, +): + with pytest.raises(RedisVLMCPError) as exc_info: + await search_records(multi_index_server, query="science", index="missing") + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + + +@pytest.mark.asyncio +async def test_search_records_single_binding_echoes_index_when_omitted(started_server): + server = await started_server({"type": "vector"}) + + response = await search_records(server, query="science") + + assert response["index"] == "knowledge" + + @pytest.mark.asyncio async def test_search_records_vector_success_with_pagination_and_projection( started_server, diff --git a/tests/unit/test_mcp/test_search_tool_unit.py b/tests/unit/test_mcp/test_search_tool_unit.py index aaeae595..60a887f5 100644 --- a/tests/unit/test_mcp/test_search_tool_unit.py +++ b/tests/unit/test_mcp/test_search_tool_unit.py @@ -111,8 +111,16 @@ def __init__( self.vectorizer = FakeVectorizer() if include_vectorizer else None self.registered_tools = [] self.native_hybrid_supported = False + self.resolved_index_ids: list[str | None] = [] def resolve_binding(self, index_id=None): + self.resolved_index_ids.append(index_id) + if index_id is not None and index_id != "knowledge": + raise RedisVLMCPError( + f"Unknown index '{index_id}'; available: knowledge", + code=MCPErrorCode.INVALID_REQUEST, + retryable=False, + ) return BindingRuntime( binding_id="knowledge", binding=self.config.indexes["knowledge"], @@ -313,6 +321,7 @@ async def fake_query(query): assert built_queries[0]["normalize_vector_distance"] is False assert built_queries[0]["ef_runtime"] == 42 assert response == { + "index": "knowledge", "search_type": "vector", "offset": 0, "limit": 2, @@ -759,6 +768,64 @@ def test_build_search_tool_description_preserves_schema_order_and_excludes_vecto assert "embedding" not in description.split("Allowed return_fields: ", 1)[1] +@pytest.mark.asyncio +async def test_search_records_defaults_to_sole_binding_when_index_omitted(monkeypatch): + server = FakeServer() + + async def fake_query(query): + return [] + + server.index.query = fake_query + + response = await search_records(server, query="science") + + assert server.resolved_index_ids == [None] + assert response["index"] == "knowledge" + + +@pytest.mark.asyncio +async def test_search_records_routes_to_named_index(monkeypatch): + server = FakeServer() + + async def fake_query(query): + return [] + + server.index.query = fake_query + + response = await search_records(server, query="science", index="knowledge") + + assert server.resolved_index_ids == ["knowledge"] + assert response["index"] == "knowledge" + + +@pytest.mark.asyncio +async def test_search_records_rejects_unknown_index(): + server = FakeServer() + + with pytest.raises(RedisVLMCPError) as exc_info: + await search_records(server, query="science", index="missing") + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + assert server.resolved_index_ids == ["missing"] + + +def test_register_search_tool_wrapper_exposes_index_param(): + server = FakeServer() + register_search_tool(server, server.index.schema) + + annotations = server.registered_tools[0]["fn"].__annotations__ + assert "index" in annotations + + +def test_build_search_tool_description_appends_routing_note_when_schema_is_ambiguous(): + description = _build_search_tool_description(None) + + assert "list-indexes" in description + assert "`index`" in description + # Per-field hints are omitted because the index is ambiguous. + assert "Object filter fields" not in description + + def test_build_search_tool_description_distinguishes_typed_and_exists_support(): schema = IndexSchema.from_dict( { From df5281964e81ec1669fef04caa6c237e70d066c5 Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Thu, 2 Jul 2026 13:41:16 +0200 Subject: [PATCH 4/5] feat(mcp): add index routing + per-index write policy to upsert-records (RAAE-1607) (#632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation This completes the multi-index tool surface for the RedisVL MCP server. After [RAAE-1606](https://redislabs.atlassian.net/browse/RAAE-1606) taught `search-records` to route by logical index, `upsert-records` ([RAAE-1607](https://redislabs.atlassian.net/browse/RAAE-1607)) needs the same explicit routing β€” but writes also carry a policy dimension that reads do not. A single server can now host a mix of writable and read-only bindings, and the tool must respect both the global `--read-only` override and each binding's own `read_only` flag while staying backwards-safe for existing single-index clients. The design keeps single-index behavior identical: when one binding is configured and `index` is omitted, the write resolves to that binding exactly as before. Routing becomes mandatory only once multiple bindings exist. Write enforcement happens at two complementary levels so the contract is unambiguous: a server with no writable bindings should not advertise the tool at all, while a server with some writable bindings still needs to protect the read-only ones on a per-call basis. ## Implemented changes `upsert-records` gains an optional `index` argument naming the logical binding to write to, resolved through the shared `resolve_binding` routing introduced in RAAE-1604. An omitted `index` with one binding resolves to that binding; an omitted `index` with multiple bindings returns `invalid_request`; and an unknown id returns `invalid_request`. The resolved logical id is echoed back as the `index` field of the response, and the selected binding's embedding, runtime limits, and schema validation drive the rest of the write unchanged. Write availability is enforced at two levels. The tool registration gate is refined from "global read-only is off" to "at least one binding is writable" β€” expressed via `effective_read_only`, which already folds in both global read-only mode and a binding's own `read_only` policy β€” so an all-read-only server does not expose `upsert-records` at all. When the tool is registered, a per-call guard rejects writes to any individual read-only binding with `invalid_request` *before* any embedding or backend write occurs, so a writable server can still protect specific indexes. Minor additional changes: - The FastMCP wrapper exposes `index` as a tool parameter. - Unit coverage for default-to-sole-binding, named routing, unknown-id rejection, read-only-binding rejection, the wrapper param, and both registration-gate outcomes (any-writable exposes the tool; all-read-only hides it). - Integration coverage on a two-binding server (one writable vector index, one read-only fulltext index): routing to the writable binding, omitted-index rejection, unknown-id rejection, read-only-binding rejection, and single-binding echo. ## Verification - `make format` (isort + black) and mypy clean on changed files. - Full MCP suite: **244 passed, 2 skipped** (Redis-version-gated) across unit + integration. ## Stacking This PR targets `feature/raae-1606-search-routing` so its diff stays scoped to upsert routing + write policy. Review/merge bottom-up: [#629](https://github.com/redis/redis-vl-python/pull/629) β†’ [#630](https://github.com/redis/redis-vl-python/pull/630) β†’ [#631](https://github.com/redis/redis-vl-python/pull/631) β†’ this PR. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1606]: https://redislabs.atlassian.net/browse/RAAE-1606?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1607]: https://redislabs.atlassian.net/browse/RAAE-1607?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > **Medium Risk** > Changes write routing and when the upsert tool is exposed on multi-index servers; mistakes could allow writes to the wrong index or hide/show the tool unexpectedly, though enforcement is fail-closed before backend writes. > > **Overview** > **`upsert-records`** now accepts an optional **`index`** logical binding id (via shared **`resolve_binding`**), matching multi-index **`search-records`**: omit **`index`** when one binding is configured; require it when several exist; reject unknown ids. Successful responses include an **`index`** field naming the binding that was written. > > Write policy is split between **tool advertisement** and **per-call enforcement**. **`upsert-records`** is registered only when at least one binding is writable (**`effective_read_only`** is false for some binding), not merely when global read-only mode is off. Each call still rejects writes to read-only bindings (**`FORBIDDEN`**, before embedding or Redis load) with a clearer message naming the binding. > > The FastMCP wrapper exposes **`index`** as a tool parameter. Unit and integration tests cover routing, registration gating, and read-only rejection. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 98aded71a950a23a3173d08c9f33d1205e0da8bb. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). --------- Co-authored-by: Claude Opus 4.8 (1M context) --- redisvl/mcp/server.py | 6 +- redisvl/mcp/tools/upsert.py | 23 +++- .../integration/test_mcp/test_upsert_tool.py | 124 ++++++++++++++++++ tests/unit/test_mcp/test_server_unit.py | 61 ++++++++- tests/unit/test_mcp/test_upsert_tool_unit.py | 58 +++++++- 5 files changed, 261 insertions(+), 11 deletions(-) diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index 899bb6f7..d3d03491 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -250,7 +250,11 @@ def _register_tools(self) -> None: # Discovery is always available so clients can enumerate indexes. register_list_indexes_tool(self) register_search_tool(self, search_schema) - if not self.mcp_settings.read_only: + # Expose upsert only when at least one binding is writable. A binding is + # read-only under global read-only mode or its own read_only policy, both + # of which are folded into effective_read_only; the per-call write check + # in the tool then rejects writes to any individual read-only binding. + if any(not rt.effective_read_only for rt in self._bindings.values()): register_upsert_tool(self) self._tools_registered = True diff --git a/redisvl/mcp/tools/upsert.py b/redisvl/mcp/tools/upsert.py index c0d3b7cc..4137c9a1 100644 --- a/redisvl/mcp/tools/upsert.py +++ b/redisvl/mcp/tools/upsert.py @@ -249,19 +249,27 @@ async def upsert_records( server: Any, *, records: list[dict[str, Any]], + index: str | None = None, id_field: str | None = None, skip_embedding_if_present: bool | None = None, ) -> dict[str, Any]: - """Execute `upsert-records` against the selected Redis index binding.""" + """Execute `upsert-records` against the selected Redis index binding. + + ``index`` names the logical binding to write to. It is optional when exactly + one binding is configured and required when multiple exist. Writes to a + read-only binding (whether from global read-only mode or the binding's own + ``read_only`` policy) are rejected with ``invalid_request``. The resolved + logical id is echoed back in the response. + """ try: - rt = server.resolve_binding(None) + rt = server.resolve_binding(index) if rt.effective_read_only: raise RedisVLMCPError( - "upsert-records is not permitted: binding is read-only", + f"index '{rt.binding_id}' is read-only", code=MCPErrorCode.FORBIDDEN, retryable=False, ) - index = rt.index + index_obj = rt.index runtime = rt.binding.runtime effective_skip_embedding = _validate_request( runtime=runtime, @@ -275,7 +283,7 @@ async def upsert_records( for record in prepared_records: _validate_record( record, - index=index, + index=index_obj, vector_field_name=runtime.vector_field_name, ) if rt.binding.supports_server_side_embedding: @@ -332,7 +340,7 @@ async def upsert_records( try: keys = await server.run_guarded( "upsert-records", - index.load(loadable_records, id_field=id_field), + index_obj.load(loadable_records, id_field=id_field), timeout_seconds=runtime.request_timeout_seconds, ) except Exception as exc: @@ -341,6 +349,7 @@ async def upsert_records( raise mapped from exc return { + "index": rt.binding_id, "status": "success", "keys_upserted": len(keys), "keys": keys, @@ -359,6 +368,7 @@ def register_upsert_tool(server: Any) -> None: async def upsert_records_tool( records: list[dict[str, Any]], + index: str | None = None, id_field: str | None = None, skip_embedding_if_present: bool | None = None, ): @@ -369,6 +379,7 @@ async def upsert_records_tool( return await upsert_records( server, records=records, + index=index, id_field=id_field, skip_embedding_if_present=skip_embedding_if_present, ) diff --git a/tests/integration/test_mcp/test_upsert_tool.py b/tests/integration/test_mcp/test_upsert_tool.py index ec08d358..a723b30e 100644 --- a/tests/integration/test_mcp/test_upsert_tool.py +++ b/tests/integration/test_mcp/test_upsert_tool.py @@ -357,6 +357,130 @@ async def fail_load(*args: Any, **kwargs: Any) -> Any: assert called is False +@pytest.fixture +async def multi_index_upsert_server( + monkeypatch, upsertable_index, fulltext_only_upsert_index, tmp_path, redis_url +): + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: RecordingVectorizer, + ) + + config = { + "server": {"redis_url": redis_url}, + "indexes": { + "knowledge": { + "redis_name": upsertable_index.schema.index.name, + "search": {"type": "vector"}, + "vectorizer": { + "class": "RecordingVectorizer", + "model": "fake-model", + "dims": 3, + }, + "runtime": { + "text_field_name": "content", + "vector_field_name": "embedding", + "default_embed_text_field": "content", + "default_limit": 2, + "max_limit": 5, + "max_upsert_records": 64, + "skip_embedding_if_present": True, + }, + }, + "tickets": { + "redis_name": fulltext_only_upsert_index.schema.index.name, + "read_only": True, + "search": {"type": "fulltext", "params": {"stopwords": None}}, + "runtime": { + "text_field_name": "content", + "vector_field_name": None, + "default_embed_text_field": None, + "default_limit": 2, + "max_limit": 5, + "max_upsert_records": 64, + }, + }, + }, + } + config_path = tmp_path / "multi-index-upsert.yaml" + config_path.write_text(yaml.safe_dump(config), encoding="utf-8") + + server = RedisVLMCPServer(MCPSettings(config=str(config_path))) + await server.startup() + try: + yield server + finally: + await server.shutdown() + + +@pytest.mark.asyncio +async def test_upsert_records_routes_to_named_writable_binding( + multi_index_upsert_server, +): + response = await upsert_records( + multi_index_upsert_server, + index="knowledge", + records=[{"content": "routed document", "category": "science", "rating": 5}], + ) + + assert response["index"] == "knowledge" + assert response["status"] == "success" + assert response["keys_upserted"] == 1 + + +@pytest.mark.asyncio +async def test_upsert_records_requires_index_when_multiple_bindings( + multi_index_upsert_server, +): + with pytest.raises(RedisVLMCPError) as exc_info: + await upsert_records( + multi_index_upsert_server, + records=[{"content": "no index", "category": "science"}], + ) + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + + +@pytest.mark.asyncio +async def test_upsert_records_rejects_unknown_index_on_multi_binding( + multi_index_upsert_server, +): + with pytest.raises(RedisVLMCPError) as exc_info: + await upsert_records( + multi_index_upsert_server, + index="missing", + records=[{"content": "doc", "category": "science"}], + ) + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + + +@pytest.mark.asyncio +async def test_upsert_records_rejects_writes_to_read_only_binding( + multi_index_upsert_server, +): + with pytest.raises(RedisVLMCPError, match="read-only") as exc_info: + await upsert_records( + multi_index_upsert_server, + index="tickets", + records=[{"content": "doc", "category": "operations"}], + ) + + assert exc_info.value.code == MCPErrorCode.FORBIDDEN + + +@pytest.mark.asyncio +async def test_upsert_records_single_binding_echoes_index_when_omitted(started_server): + server = await started_server() + + response = await upsert_records( + server, + records=[{"content": "solo document", "category": "science", "rating": 5}], + ) + + assert response["index"] == "knowledge" + + @pytest.mark.asyncio async def test_read_only_mode_excludes_upsert_tool( monkeypatch, upsertable_index, mcp_config_path diff --git a/tests/unit/test_mcp/test_server_unit.py b/tests/unit/test_mcp/test_server_unit.py index 2aa73473..0bd580d9 100644 --- a/tests/unit/test_mcp/test_server_unit.py +++ b/tests/unit/test_mcp/test_server_unit.py @@ -53,7 +53,9 @@ async def test_probe_native_hybrid_search_false_for_old_redis_py(monkeypatch): assert client.info_calls == 0 -def _binding_runtime(binding_id: str) -> BindingRuntime: +def _binding_runtime( + binding_id: str, *, effective_read_only: bool = False +) -> BindingRuntime: return BindingRuntime( binding_id=binding_id, binding=SimpleNamespace(), @@ -61,7 +63,7 @@ def _binding_runtime(binding_id: str) -> BindingRuntime: schema=SimpleNamespace(), vectorizer=None, supports_native_hybrid_search=False, - effective_read_only=False, + effective_read_only=effective_read_only, ) @@ -140,3 +142,58 @@ async def fake_close_resources(self, *, index, vectorizer): # ...but tool registration is instance-level and must survive teardown, so a # stop/start does not re-register the same tool names on the FastMCP object. assert server._tools_registered is True + + +def _register_tools_with(monkeypatch, bindings: dict) -> list[str]: + """Run _register_tools against the given bindings, returning registered names.""" + registered: list[str] = [] + monkeypatch.setattr( + "redisvl.mcp.server.register_list_indexes_tool", + lambda server: registered.append("list-indexes"), + ) + monkeypatch.setattr( + "redisvl.mcp.server.register_search_tool", + lambda server, schema: registered.append("search-records"), + ) + monkeypatch.setattr( + "redisvl.mcp.server.register_upsert_tool", + lambda server: registered.append("upsert-records"), + ) + + server = RedisVLMCPServer.__new__(RedisVLMCPServer) + server._bindings = bindings + server._tools_registered = False + server.tool = object() + server.mcp_settings = SimpleNamespace(read_only=False) + + server._register_tools() + return registered + + +def test_register_tools_exposes_upsert_when_a_binding_is_writable(monkeypatch): + registered = _register_tools_with( + monkeypatch, + { + "knowledge": _binding_runtime("knowledge", effective_read_only=False), + "tickets": _binding_runtime("tickets", effective_read_only=True), + }, + ) + + assert "upsert-records" in registered + assert "list-indexes" in registered + assert "search-records" in registered + + +def test_register_tools_hides_upsert_when_every_binding_is_read_only(monkeypatch): + registered = _register_tools_with( + monkeypatch, + { + "knowledge": _binding_runtime("knowledge", effective_read_only=True), + "tickets": _binding_runtime("tickets", effective_read_only=True), + }, + ) + + assert "upsert-records" not in registered + # Read paths stay available even when writes are globally disabled. + assert "list-indexes" in registered + assert "search-records" in registered diff --git a/tests/unit/test_mcp/test_upsert_tool_unit.py b/tests/unit/test_mcp/test_upsert_tool_unit.py index 5c2e059d..2327af90 100644 --- a/tests/unit/test_mcp/test_upsert_tool_unit.py +++ b/tests/unit/test_mcp/test_upsert_tool_unit.py @@ -166,8 +166,16 @@ def __init__( self.vectorizer = vectorizer or FakeVectorizer() if include_vectorizer else None self.registered_tools = [] self.effective_read_only = effective_read_only + self.resolved_index_ids: list[str | None] = [] def resolve_binding(self, index_id=None): + self.resolved_index_ids.append(index_id) + if index_id is not None and index_id != "knowledge": + raise RedisVLMCPError( + f"Unknown index '{index_id}'; available: knowledge", + code=MCPErrorCode.INVALID_REQUEST, + retryable=False, + ) return BindingRuntime( binding_id="knowledge", binding=self.config.indexes["knowledge"], @@ -212,6 +220,7 @@ async def test_upsert_records_generates_missing_vectors_and_serializes_hash_vect ) assert response == { + "index": "knowledge", "status": "success", "keys_upserted": 2, "keys": ["doc:alpha", "doc:beta"], @@ -457,16 +466,61 @@ async def test_upsert_records_surfaces_partial_write_possible_on_backend_failure assert isinstance(exc_info.value.__cause__, RedisError) +@pytest.mark.asyncio +async def test_upsert_records_defaults_to_sole_binding_when_index_omitted(): + server = FakeServer() + + response = await upsert_records(server, records=[{"content": "alpha doc"}]) + + assert server.resolved_index_ids == [None] + assert response["index"] == "knowledge" + + +@pytest.mark.asyncio +async def test_upsert_records_routes_to_named_index(): + server = FakeServer() + + response = await upsert_records( + server, records=[{"content": "alpha doc"}], index="knowledge" + ) + + assert server.resolved_index_ids == ["knowledge"] + assert response["index"] == "knowledge" + + +@pytest.mark.asyncio +async def test_upsert_records_rejects_unknown_index(): + server = FakeServer() + + with pytest.raises(RedisVLMCPError) as exc_info: + await upsert_records( + server, records=[{"content": "alpha doc"}], index="missing" + ) + + assert exc_info.value.code == MCPErrorCode.INVALID_REQUEST + assert server.resolved_index_ids == ["missing"] + assert server.index.load_calls == [] + + @pytest.mark.asyncio async def test_upsert_records_rejects_writes_to_read_only_binding(): server = FakeServer(effective_read_only=True) - with pytest.raises(RedisVLMCPError) as exc_info: + with pytest.raises(RedisVLMCPError, match="read-only") as exc_info: await upsert_records(server, records=[{"content": "alpha doc"}]) assert exc_info.value.code == MCPErrorCode.FORBIDDEN - # The write is rejected before any backend load is attempted. + # Write policy is enforced before any embedding or backend write. assert server.index.load_calls == [] + assert server.vectorizer.aembed_many_calls == [] + + +def test_register_upsert_tool_wrapper_exposes_index_param(): + server = FakeServer() + register_upsert_tool(server) + + annotations = server.registered_tools[0]["fn"].__annotations__ + assert "index" in annotations def test_register_upsert_tool_uses_default_and_override_descriptions(): From a55b751c8e0b34a7c87e4c7d1486c5fd7675ffe0 Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Thu, 2 Jul 2026 15:37:31 +0200 Subject: [PATCH 5/5] docs(mcp): document multi-index server contract (RAAE-1608) (#633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation The RedisVL MCP docs were written for the original single-index model and still stated that "one server process binds to exactly one existing Redis index." With multi-index support now implemented across [RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)–[RAAE-1607](https://redislabs.atlassian.net/browse/RAAE-1607), the user-facing documentation ([RAAE-1608](https://redislabs.atlassian.net/browse/RAAE-1608)) needs to describe the new compatibility story without confusing existing single-index users. The guiding principle in the rewrite is that single-index remains the simplest deployment and works exactly as before β€” callers never name an index β€” while multi-index is presented as a formal, additive capability layered on top via discovery (`list-indexes`) and explicit routing (the `index` argument). No documentation still claims a server must bind to exactly one index. ## Implemented changes The concept doc ([concepts/mcp.md](docs/concepts/mcp.md)) now frames the server as binding one *or several* logical indexes, each addressed by an id, and adds an "Index Selection and Discovery" section covering the optional `index` argument, the omitted-index and unknown-id rules, the response echo, and discovery-first guidance. The "Single Index Binding" section becomes "Single and Multiple Index Bindings," the read-only section explains the two-level write policy (global `--read-only` vs per-index `read_only`, folded into effective write availability), and the tool surface gains a `list-indexes` subsection documenting its minimal payload β€” filterable fields only, vector/embed-source fields omitted, explicit-only limits, and `redis_name` never exposed. The how-to guide ([how_to_guides/mcp.md](docs/user_guide/how_to_guides/mcp.md)) adds a two-binding config example (a writable vector index alongside a read-only fulltext index), an Index Selection subsection, a `list-indexes` tool contract with a response example, and threads the optional `index` argument through the `search-records` and `upsert-records` argument lists and request/response examples. A discovery-first multi-index flow is shown at the top of the search examples, and the CLI/env-var notes are updated for per-index read-only and the multi-index search description. Minor additional changes: - README MCP section and feature-table entry reworded from "an existing Redis index" to "one or more existing Redis indexes," with `list-indexes` and the discovery-first flow noted. (The README edit was explicitly authorized for this ticket, overriding the repo's default no-README-edits rule.) ## Verification - `sphinx-build` (the `docs` dependency group) completes cleanly (exit 0). The only warnings are pre-existing and unrelated (upstream `redis-py` docstrings and `index.md` heading levels); no warnings reference the MCP pages, and no broken cross-reference/anchor warnings were introduced. ## Stacking This PR targets `feature/raae-1607-upsert-routing` and is the top of the stack. Review/merge bottom-up: [#629](https://github.com/redis/redis-vl-python/pull/629) β†’ [#630](https://github.com/redis/redis-vl-python/pull/630) β†’ [#631](https://github.com/redis/redis-vl-python/pull/631) β†’ [#632](https://github.com/redis/redis-vl-python/pull/632) β†’ this PR. πŸ€– Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1607]: https://redislabs.atlassian.net/browse/RAAE-1607?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1608]: https://redislabs.atlassian.net/browse/RAAE-1608?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > **Low Risk** > Markdown-only changes with no runtime, API, or security behavior modified. > > **Overview** > **Documentation-only** update aligning RedisVL MCP user-facing docs with multi-index support: one server can bind **one or several** logical indexes while **single-index deployments stay backward compatible** (omit `index`). > > **README** rewords MCP copy from a single index to one or more, and notes **`list-indexes`**, discovery-first routing, and per-index tools. > > **`docs/concepts/mcp.md`** reframes the model (per-index config, all-or-nothing startup), adds **Index Selection and Discovery** (`index` argument rules, response echo), expands **read-only** to global vs per-index `read_only` and effective `upsert_available`, and documents the **`list-indexes`** payload (minimal fields, no `redis_name`). > > **`docs/user_guide/how_to_guides/mcp.md`** adds a **two-binding YAML example**, **`list-indexes`** contract and examples, threads optional **`index`** through search/upsert docs, a discovery-first flow, and updates CLI/env notes for multi-index search tool descriptions. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit de2eb4fe782b38d3b452715207373ffc8420cae2. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). Co-authored-by: Claude Opus 4.8 (1M context) --- README.md | 16 +-- docs/concepts/mcp.md | 81 ++++++++++----- docs/user_guide/how_to_guides/mcp.md | 146 ++++++++++++++++++++++++++- 3 files changed, 209 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 55f790cd..426e7cf0 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Perfect for building **RAG pipelines** with real-time retrieval, **AI agents** w | **[Vector Search](#retrieval)**
*Similarity search with metadata filters* | **[LLM Memory](#llm-memory)**
*Agentic AI context management* | **Async Support**
*Async indexing and search for improved performance* | | **[Complex Filtering](#retrieval)**
*Combine multiple filter types* | **[Semantic Routing](#semantic-routing)**
*Intelligent query classification* | **[Vectorizers](#vectorizers)**
*8+ embedding provider integrations* | | **[Hybrid Search](#retrieval)**
*Combine semantic & full-text signals* | **[Embedding Caching](#embedding-caching)**
*Cache embeddings for efficiency* | **[Rerankers](#rerankers)**
*Improve search result relevancy* | -| | | **[MCP Server](#mcp-server)**
*Expose an existing Redis index to MCP clients* | +| | | **[MCP Server](#mcp-server)**
*Expose one or more existing Redis indexes to MCP clients* | @@ -51,7 +51,7 @@ Install `redisvl` into your Python (>=3.10) environment using `pip`: pip install redisvl ``` -Install the MCP server extra when you want to expose an existing Redis index through MCP: +Install the MCP server extra when you want to expose one or more existing Redis indexes through MCP: ```bash pip install redisvl[mcp] @@ -572,16 +572,18 @@ Use `--read-only` to expose search without upsert. ### MCP Server -RedisVL includes an MCP server that lets MCP-compatible clients search or upsert data in an existing Redis index through a small, stable tool contract. +RedisVL includes an MCP server that lets MCP-compatible clients search or upsert data in one or more existing Redis indexes through a small, stable tool contract. The server: -- connects to one existing Redis Search index -- reconstructs the schema from Redis at startup -- uses the configured vectorizer for query embedding and optional upsert embedding -- exposes `search-records` and, unless read-only mode is enabled, `upsert-records` +- connects to one or more existing Redis Search indexes, each addressed by a logical id +- reconstructs each index's schema from Redis at startup +- uses each index's configured vectorizer for query embedding and optional upsert embedding +- exposes `list-indexes` for discovery, `search-records`, and (unless every index is read-only) `upsert-records` - supports stdio (default), Streamable HTTP, and SSE transports +A single configured index is the simplest case and works exactly as before β€” callers omit the index selector. With multiple indexes, clients call `list-indexes` first and pass the chosen `index` to `search-records` and `upsert-records`. + Run it over stdio (default): ```bash diff --git a/docs/concepts/mcp.md b/docs/concepts/mcp.md index e6d44011..ff192d26 100644 --- a/docs/concepts/mcp.md +++ b/docs/concepts/mcp.md @@ -7,28 +7,30 @@ myst: # RedisVL MCP -RedisVL includes an MCP server that exposes a Redis-backed retrieval surface through a small, deterministic tool contract. It is designed for AI applications that want to search or maintain data in an existing Redis index without each client reimplementing Redis query logic. +RedisVL includes an MCP server that exposes a Redis-backed retrieval surface through a small, deterministic tool contract. It is designed for AI applications that want to search or maintain data in one or more existing Redis indexes without each client reimplementing Redis query logic. ## What RedisVL MCP Does The RedisVL MCP server sits between an MCP client and Redis: -1. It connects to an existing Redis Search index. -2. It inspects that index at startup and reconstructs its schema. +1. It connects to one or more existing Redis Search indexes. +2. It inspects each index at startup and reconstructs its schema. 3. It initializes vector capabilities only when the configured search or upsert behavior needs them. -4. It exposes stable MCP tools for search, and optionally upsert. +4. It exposes stable MCP tools for discovery, search, and optionally upsert. -This keeps the Redis index as the source of truth for search behavior while giving MCP clients a predictable interface. +This keeps each Redis index as the source of truth for its search behavior while giving MCP clients a predictable interface. ## How RedisVL MCP Runs RedisVL MCP works with a focused model: -- One server process binds to exactly one existing Redis index. +- One server process binds to one *or several* existing Redis indexes, each addressed by a logical id. - The server supports stdio (default), Streamable HTTP, and SSE transports. -- Search behavior is owned by configuration, not by MCP callers. -- Vector search and server-side embedding are optional capabilities configured explicitly. -- Upsert is optional and can be disabled with read-only mode. +- Search behavior is owned by per-index configuration, not by MCP callers. +- Vector search and server-side embedding are optional capabilities configured explicitly per index. +- Upsert is optional and can be disabled globally with read-only mode or per index with a `read_only` flag. + +A single-index server remains the simplest deployment: when exactly one index is configured, callers can omit the index selector entirely and every tool call targets that index. Multi-index support is fully formal β€” it adds discovery and explicit routing without changing the single-index contract. ## Config-Owned Search Behavior @@ -44,17 +46,30 @@ These request-time controls are still bounded by runtime config. In particular, deep paging is limited by a configured maximum result window, enforced as `offset + limit`. -MCP callers do not choose: +On a multi-index server, callers also choose **which index to target** through an optional `index` argument (see [Index Selection](#index-selection-and-discovery)). Callers do not choose: -- which index to target - whether retrieval is `vector`, `fulltext`, or `hybrid` - query tuning parameters such as hybrid fusion or vector runtime settings -That behavior lives in the server config under `indexes..search`. The response includes `search_type` as informational metadata, but it is not a request parameter. +That behavior lives in the per-index server config under `indexes..search`. The response includes `search_type` as informational metadata, but it is not a request parameter. + +## Single and Multiple Index Bindings + +The YAML config uses an `indexes` mapping. Each entry is a logical binding keyed by an id (for example `knowledge` or `tickets`) that points to an existing Redis index through `redis_name`. The mapping may contain one entry or several; each binding is inspected, validated, and given its own search config, runtime limits, and optional vectorizer independently at startup. Startup is all-or-nothing β€” if any binding fails to initialize, the server does not start. + +A single-binding config is the simplest case and behaves exactly as before: the lone binding is the implicit target of every call. With multiple bindings the server stays a single process and endpoint, but callers select a binding per call. + +## Index Selection and Discovery + +On a multi-index server, every tool call must say which logical index it targets: -## Single Index Binding +- `search-records` and `upsert-records` accept an optional `index` argument naming the logical id. +- When exactly one index is configured, `index` may be omitted and resolves to that sole binding (backward compatible). +- When multiple indexes are configured, omitting `index` is an `invalid_request`; the caller must name one. +- An unknown logical id is an `invalid_request`. +- Both tools echo the resolved `index` in their response so clients can confirm routing. -The YAML config uses an `indexes` mapping with one configured entry. That binding points to an existing Redis index through `redis_name`, and every tool call targets that configured index. +Because a client cannot guess the configured logical ids, multi-index servers expose a `list-indexes` discovery tool. **Clients should call `list-indexes` first** to enumerate the available indexes and their filterable fields, then pass the chosen id as `index` on subsequent calls. ## Schema Inspection and Overrides @@ -71,14 +86,16 @@ MCP-reserved score metadata field names for the configured search mode. ## Read-Only and Read-Write Modes -RedisVL MCP always registers `search-records`. +RedisVL MCP always registers `search-records` and `list-indexes`. -`upsert-records` is only registered when the server is not in read-only mode. Read-only mode is controlled by: +Write availability is enforced at two levels: -- the CLI flag `--read-only` -- or the environment variable `REDISVL_MCP_READ_ONLY=true` +- **Global read-only mode** disables writes across every binding. It is controlled by the CLI flag `--read-only` or the environment variable `REDISVL_MCP_READ_ONLY=true`. +- **Per-index read-only** disables writes for a single binding via `indexes..read_only: true`, while other bindings stay writable. -Use read-only mode when Redis is serving approved content to assistants and another system owns ingestion. +These combine into each binding's *effective* write availability: a binding is read-only if global read-only is on **or** that binding sets `read_only: true`. The `upsert-records` tool is registered only when at least one binding is writable, so a fully read-only server does not advertise it at all. When the tool is registered, a write to a read-only binding is rejected with `forbidden` before any data is changed. `list-indexes` reports each binding's effective write availability as `upsert_available`. + +Use read-only mode when Redis is serving approved content to assistants and another system owns ingestion β€” globally when no binding should accept writes, or per index when only some indexes are writable. ## Authentication and Authorization @@ -88,18 +105,36 @@ For configuration and the gateway boundary, see {doc}`/user_guide/how_to_guides/ ## Tool Surface -RedisVL MCP exposes two tools: +RedisVL MCP exposes up to three tools: -- `search-records` searches the configured index using the server-owned search mode -- `upsert-records` validates and upserts records, embedding them only when that capability is configured +- `list-indexes` enumerates the configured logical indexes for discovery (always available) +- `search-records` searches a selected index using that index's server-owned search mode +- `upsert-records` validates and upserts records into a selected writable index, embedding them only when that capability is configured These tools follow a stable contract: - request validation happens before query or write execution +- the resolved logical `index` is echoed in every `search-records` and `upsert-records` response - filters support either raw strings or a RedisVL-backed JSON DSL -- `search-records` describes the inspected schema by advertising typed JSON DSL filter fields, object-filter `exists` support, and valid `return_fields` +- on a single-index server, `search-records` describes the inspected schema by advertising typed JSON DSL filter fields, object-filter `exists` support, and valid `return_fields`; on a multi-index server those hints are ambiguous, so the description instead directs clients to call `list-indexes` and pass `index` - error codes are mapped into a stable set of MCP-facing categories +### `list-indexes` + +`list-indexes` returns one entry per configured binding so clients can route subsequent calls. Each entry reports: + +- the logical `id` +- an optional `description` (only when configured) +- `upsert_available`, reflecting the binding's effective write availability +- `fields`, the filterable fields discovered from the index +- `limits`, only the runtime limits that were explicitly configured + +The discovery payload is deliberately minimal: + +- the underlying Redis index name (`redis_name`) is **never** exposed +- the vector field and the configured embed-source text field are **omitted** from `fields`, since they are implementation inputs rather than fields a client filters on +- `limits` shows only explicitly set values (such as `max_limit` or `max_upsert_records`); defaults are not echoed + ## Why Use MCP Instead of Direct RedisVL Calls Use RedisVL MCP when you want a standard tool boundary for agent frameworks or assistants that already speak MCP. diff --git a/docs/user_guide/how_to_guides/mcp.md b/docs/user_guide/how_to_guides/mcp.md index b0a4132b..722074fd 100644 --- a/docs/user_guide/how_to_guides/mcp.md +++ b/docs/user_guide/how_to_guides/mcp.md @@ -71,7 +71,7 @@ uvx --from redisvl[mcp] rvl mcp --config /path/to/mcp.yaml --read-only | `--transport` | `stdio` | Transport protocol: `stdio`, `sse`, or `streamable-http` | | `--host` | `127.0.0.1` | Bind address (only used with `sse` and `streamable-http`) | | `--port` | `8000` | Bind port (only used with `sse` and `streamable-http`) | -| `--read-only` | off | Disable the `upsert-records` tool | +| `--read-only` | off | Disable writes across every index (global read-only) | ### Environment Variables @@ -81,7 +81,7 @@ You can also control boot settings through environment variables: |----------|---------| | `REDISVL_MCP_CONFIG` | Path to the MCP YAML config | | `REDISVL_MCP_READ_ONLY` | Disable `upsert-records` when set to `true` | -| `REDISVL_MCP_TOOL_SEARCH_DESCRIPTION` | Set the base search tool description text; RedisVL still appends schema-derived typed filter, `exists`, and `return_fields` hints | +| `REDISVL_MCP_TOOL_SEARCH_DESCRIPTION` | Set the base search tool description text. On a single-index server RedisVL appends schema-derived typed filter, `exists`, and `return_fields` hints; on a multi-index server it appends a note directing clients to call `list-indexes` and pass `index` | | `REDISVL_MCP_TOOL_UPSERT_DESCRIPTION` | Override the upsert tool description | ## Connect a Remote MCP Client @@ -108,7 +108,7 @@ For example, to configure a remote MCP client to connect to a Streamable HTTP se ## Example Config -This example binds one logical MCP server to one existing Redis index called `knowledge`. +This example binds one logical MCP server to one existing Redis index called `knowledge`. A single configured index is the simplest deployment, and callers never need to name it. See [Multiple Indexes](#multiple-indexes) below to expose several indexes from the same server. The config uses `${REDIS_URL}` and `${OPENAI_API_KEY}` as environment-variable placeholders. These values are resolved when the server starts. You can also use `${VAR:-default}` to provide a fallback value. @@ -198,14 +198,117 @@ indexes: max_concurrency: 16 ``` +### Multiple Indexes + +The `indexes` mapping can hold more than one binding. Each entry is keyed by a logical id, points at its own existing Redis index through `redis_name`, and carries its own `search`, `runtime`, and optional `vectorizer`. The example below exposes a writable vector index `knowledge` alongside a read-only fulltext index `tickets` from the same server: + +```yaml +server: + redis_url: ${REDIS_URL} + +indexes: + knowledge: + redis_name: knowledge + description: Internal runbooks and operational guidance. + + vectorizer: + class: OpenAITextVectorizer + model: text-embedding-3-small + api_config: + api_key: ${OPENAI_API_KEY} + + search: + type: vector + + runtime: + text_field_name: content + vector_field_name: embedding + default_embed_text_field: content + default_limit: 10 + max_limit: 25 + + tickets: + redis_name: support-tickets + description: Read-only mirror of resolved support tickets. + read_only: true + + search: + type: fulltext + params: + text_scorer: BM25STD + stopwords: english + + runtime: + text_field_name: body + default_limit: 10 + max_limit: 50 +``` + +Notes: + +- Each binding is inspected and validated independently at startup. Startup is all-or-nothing: if any binding fails, the server does not start. +- The optional per-index `description` and `read_only` flags are surfaced through `list-indexes`. +- `read_only: true` makes that binding reject writes even though the server as a whole is not in global read-only mode. Because `knowledge` is still writable, the `upsert-records` tool is registered; an upsert targeting `tickets` is rejected with `forbidden`. +- A single-index config keeps working unchanged β€” adding bindings does not change how the sole-binding case behaves. + +### Index Selection + +On a multi-index server, `search-records` and `upsert-records` take an optional `index` argument naming the logical id to target: + +- With exactly one index configured, `index` may be omitted and resolves to that binding. +- With multiple indexes configured, omitting `index` returns `invalid_request`; an unknown id also returns `invalid_request`. +- Clients should call [`list-indexes`](#list-indexes) first to discover the available ids and their filterable fields. + ## Tool Contracts RedisVL MCP exposes a small, implementation-owned contract. +### `list-indexes` + +`list-indexes` is always available and takes no arguments. Call it first on a multi-index server to discover which logical ids exist and how to filter each one. + +Example response payload: + +```json +{ + "indexes": [ + { + "id": "knowledge", + "description": "Internal runbooks and operational guidance.", + "upsert_available": true, + "fields": [ + { "name": "title", "type": "text" }, + { "name": "category", "type": "tag" }, + { "name": "rating", "type": "numeric" } + ], + "limits": { "max_limit": 25 } + }, + { + "id": "tickets", + "description": "Read-only mirror of resolved support tickets.", + "upsert_available": false, + "fields": [ + { "name": "category", "type": "tag" } + ], + "limits": { "max_limit": 50 } + } + ] +} +``` + +Notes: + +- `upsert_available` reflects the binding's effective write availability (global read-only **or** the per-index `read_only` flag) +- `fields` lists the filterable fields discovered from the index; the vector field and the configured embed-source text field are intentionally omitted +- `limits` includes only runtime limits that were explicitly configured (such as `max_limit` or `max_upsert_records`); defaults are not echoed +- the underlying Redis index name (`redis_name`) is never exposed +- `description` appears only when configured for that binding + ### `search-records` Arguments: +- `index` (optional; required when multiple indexes are configured) - `query` - `limit` - `offset` @@ -216,6 +319,7 @@ Example request payload: ```json { + "index": "knowledge", "query": "incident response runbook", "limit": 2, "offset": 0, @@ -233,6 +337,7 @@ Example response payload: ```json { + "index": "knowledge", "search_type": "hybrid", "offset": 0, "limit": 2, @@ -254,6 +359,7 @@ Example response payload: Notes: +- `index` selects the logical binding; omit it only on a single-index server. The resolved id is echoed back in the response - `search_type` is response metadata, not a request argument - when `return_fields` is omitted, RedisVL MCP returns all non-vector fields - returning the configured vector field is rejected @@ -267,6 +373,7 @@ Notes: Arguments: +- `index` (optional; required when multiple indexes are configured) - `records` - `id_field` - `skip_embedding_if_present` @@ -275,6 +382,7 @@ Example request payload: ```json { + "index": "knowledge", "records": [ { "doc_id": "doc-42", @@ -291,6 +399,7 @@ Example response payload: ```json { + "index": "knowledge", "status": "success", "keys_upserted": 1, "keys": ["knowledge:doc-42"] @@ -299,13 +408,42 @@ Example response payload: Notes: -- this tool is not registered in read-only mode +- `index` selects the logical binding; omit it only on a single-index server. The resolved id is echoed back in the response +- this tool is not registered when every binding is read-only (global read-only mode or every binding setting `read_only: true`) +- a write targeting a read-only binding is rejected with `forbidden` before any data is changed, even when the tool is registered because other bindings are writable - when server-side embedding is configured, records that need embedding must contain `runtime.default_embed_text_field` - when `skip_embedding_if_present` is `true`, records that already contain the configured vector field can skip re-embedding - when a vector field is configured but server-side embedding is disabled, callers must supply vectors explicitly ## Search Examples +### Discovery-First Multi-Index Flow + +On a multi-index server, call `list-indexes` first, pick a logical id from the response, then pass it as `index`: + +```json +{ + "index": "knowledge", + "query": "cache invalidation incident", + "limit": 3, + "return_fields": ["title", "content", "category"] +} +``` + +The same `index` argument routes an `upsert-records` write to a specific binding: + +```json +{ + "index": "knowledge", + "records": [ + { "doc_id": "doc-7", "content": "New runbook entry", "category": "operations" } + ], + "id_field": "doc_id" +} +``` + +On a single-index server you can omit `index` entirely; the examples below show that backward-compatible shape. + ### Read-Only Vector Search Use read-only mode when assistants should only retrieve data: