diff --git a/README.md b/README.md index 55f790cdc..426e7cf07 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 e6d440116..ff192d266 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 2d75e469f..44fdcf053 100644 --- a/docs/user_guide/how_to_guides/mcp.md +++ b/docs/user_guide/how_to_guides/mcp.md @@ -94,7 +94,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 @@ -104,7 +104,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 | | `REDISVL_MCP_ALLOWED_HOSTS` | Comma-separated extra `Host` header values to accept on HTTP transports (in addition to the bind-derived defaults) | | `REDISVL_MCP_ALLOWED_ORIGINS` | Comma-separated browser `Origin` values to accept on HTTP transports | @@ -135,7 +135,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. @@ -225,14 +225,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` @@ -243,6 +346,7 @@ Example request payload: ```json { + "index": "knowledge", "query": "incident response runbook", "limit": 2, "offset": 0, @@ -260,6 +364,7 @@ Example response payload: ```json { + "index": "knowledge", "search_type": "hybrid", "offset": 0, "limit": 2, @@ -281,6 +386,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 @@ -294,6 +400,7 @@ Notes: Arguments: +- `index` (optional; required when multiple indexes are configured) - `records` - `id_field` - `skip_embedding_if_present` @@ -302,6 +409,7 @@ Example request payload: ```json { + "index": "knowledge", "records": [ { "doc_id": "doc-42", @@ -318,6 +426,7 @@ Example response payload: ```json { + "index": "knowledge", "status": "success", "keys_upserted": 1, "keys": ["knowledge:doc-42"] @@ -326,13 +435,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: diff --git a/redisvl/mcp/config.py b/redisvl/mcp/config.py index c2cd3b46a..90fb475a0 100644 --- a/redisvl/mcp/config.py +++ b/redisvl/mcp/config.py @@ -312,9 +312,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 @@ -385,83 +392,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. @@ -508,7 +441,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( @@ -605,6 +538,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 000000000..2564d8999 --- /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 d1bc2b6ba..e9dc15093 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,8 +11,11 @@ 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.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.mcp.transport_security import ( @@ -21,6 +25,8 @@ 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: @@ -51,17 +57,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 @@ -125,12 +129,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 @@ -147,22 +155,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: @@ -175,8 +214,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") @@ -184,33 +222,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 @@ -220,34 +257,39 @@ 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) - if not self.mcp_settings.read_only: + # 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 + + # Discovery is always available so clients can enumerate indexes. + register_list_indexes_tool(self) + register_search_tool(self, search_schema) + # 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 @@ -276,15 +318,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) @@ -294,12 +336,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.""" @@ -331,6 +398,7 @@ async def _begin_shutdown(self) -> bool: ): self.config = None self._semaphore = None + self._bindings = {} self._lifecycle_state = _LifecycleState.STOPPED return True @@ -364,29 +432,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: @@ -403,37 +504,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/list_indexes.py b/redisvl/mcp/tools/list_indexes.py new file mode 100644 index 000000000..58a6adefc --- /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/redisvl/mcp/tools/search.py b/redisvl/mcp/tools/search.py index 89cfa00fb..adbeab752 100644 --- a/redisvl/mcp/tools/search.py +++ b/redisvl/mcp/tools/search.py @@ -51,10 +51,20 @@ 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 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 + " 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()] @@ -79,18 +89,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 +133,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 +232,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 +326,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 +386,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, @@ -418,25 +430,31 @@ 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 configured 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: - index = await server.get_index() + rt = server.resolve_binding(index) 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,10 +463,12 @@ 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 { + "index": rt.binding_id, "search_type": search_type, "offset": offset, "limit": effective_limit, @@ -467,7 +487,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, @@ -476,17 +496,20 @@ def register_search_tool(server: Any, schema: IndexSchema) -> 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, 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, query=query, + index=index, limit=limit, offset=offset, filter=filter, diff --git a/redisvl/mcp/tools/upsert.py b/redisvl/mcp/tools/upsert.py index dd5721e8f..4137c9a1a 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 @@ -251,14 +249,30 @@ 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 configured Redis index.""" + """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: - index = await server.get_index() + rt = server.resolve_binding(index) + if rt.effective_read_only: + raise RedisVLMCPError( + f"index '{rt.binding_id}' is read-only", + code=MCPErrorCode.FORBIDDEN, + retryable=False, + ) + index_obj = 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 +280,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, + index=index_obj, 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 +302,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 +334,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), + index_obj.load(loadable_records, id_field=id_field), + timeout_seconds=runtime.request_timeout_seconds, ) except Exception as exc: mapped = map_exception(exc) @@ -334,6 +349,7 @@ async def upsert_records( raise mapped from exc return { + "index": rt.binding_id, "status": "success", "keys_upserted": len(keys), "keys": keys, @@ -352,15 +368,18 @@ 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, ): """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, records=records, + index=index, id_field=id_field, skip_embedding_if_present=skip_embedding_if_present, ) diff --git a/tests/conftest.py b/tests/conftest.py index c3ac61bab..cd09ef5d4 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_search_tool.py b/tests/integration/test_mcp/test_search_tool.py index a59f11c9b..03824fe64 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/integration/test_mcp/test_server_startup.py b/tests/integration/test_mcp/test_server_startup.py index 6b9c315a5..ec7de37e6 100644 --- a/tests/integration/test_mcp/test_server_startup.py +++ b/tests/integration/test_mcp/test_server_startup.py @@ -6,11 +6,17 @@ 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.mcp.tools.list_indexes import list_indexes 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 +139,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 +160,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 +172,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 +190,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 +219,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 +294,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 +396,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 +422,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 +439,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 +471,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 +481,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 +529,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 +541,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 +580,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 +601,208 @@ 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 == {} + + +@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/integration/test_mcp/test_transport_auth.py b/tests/integration/test_mcp/test_transport_auth.py index a31d84f7e..9c2161e6f 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 6711790b9..a723b30e4 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, ) @@ -356,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_config.py b/tests/unit/test_mcp/test_config.py index 7631e368c..512af828a 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_list_indexes_tool_unit.py b/tests/unit/test_mcp/test_list_indexes_tool_unit.py new file mode 100644 index 000000000..0d56b0833 --- /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_search_tool_unit.py b/tests/unit/test_mcp/test_search_tool_unit.py index d5a922445..60a887f5c 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, @@ -110,16 +111,27 @@ 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"], + index=self.index, + schema=self.index.schema, + vectorizer=self.vectorizer, + supports_native_hybrid_search=self.native_hybrid_supported, + effective_read_only=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 - - 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): @@ -309,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, @@ -652,7 +665,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 +714,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, ) @@ -755,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( { diff --git a/tests/unit/test_mcp/test_server.py b/tests/unit/test_mcp/test_server.py index e88b8a38f..9179bdd5b 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 ) @@ -323,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, diff --git a/tests/unit/test_mcp/test_server_unit.py b/tests/unit/test_mcp/test_server_unit.py index 13ba23d58..0bd580d98 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,169 @@ 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, *, effective_read_only: bool = False +) -> BindingRuntime: + return BindingRuntime( + binding_id=binding_id, + binding=SimpleNamespace(), + index=SimpleNamespace(), + schema=SimpleNamespace(), + vectorizer=None, + supports_native_hybrid_search=False, + effective_read_only=effective_read_only, + ) + + +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 + + +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 15d3d5cab..2327af903 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,30 @@ 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 + 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"], + 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): @@ -204,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"], @@ -449,6 +466,63 @@ 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, match="read-only") as exc_info: + await upsert_records(server, records=[{"content": "alpha doc"}]) + + assert exc_info.value.code == MCPErrorCode.FORBIDDEN + # 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(): default_server = FakeServer() register_upsert_tool(default_server)