-
Notifications
You must be signed in to change notification settings - Fork 75
feat(MCP): Implement the base framework for a RedisVL MCP server #532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
vishal-bala
wants to merge
6
commits into
feat/RAAE-1395-redisvl-mcp
Choose a base branch
from
feat/RAAE-1396/mcp-framework
base: feat/RAAE-1395-redisvl-mcp
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2929a97
RAAE-1396: Create MCP framework for RedisVL
vishal-bala cf0f089
Ensure server shutdown disconnects
vishal-bala b103a8b
Exclude MCP module from import sanity-check
vishal-bala e9518d6
Ensure startup failures clean up server resources
vishal-bala 8f98c5c
Clear index state after MCP server shutdown
vishal-bala 6b17caa
Fix MCP vectorizer cleanup on shutdown failure
vishal-bala File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| from redisvl.mcp.config import MCPConfig, load_mcp_config | ||
| from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError, map_exception | ||
| from redisvl.mcp.server import RedisVLMCPServer | ||
| from redisvl.mcp.settings import MCPSettings | ||
|
|
||
| __all__ = [ | ||
| "MCPConfig", | ||
| "MCPErrorCode", | ||
| "MCPSettings", | ||
| "RedisVLMCPError", | ||
| "RedisVLMCPServer", | ||
| "load_mcp_config", | ||
| "map_exception", | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| import os | ||
| import re | ||
| from pathlib import Path | ||
| from typing import Any, Dict, List, Optional, Union | ||
|
|
||
| import yaml | ||
| from pydantic import BaseModel, ConfigDict, Field, model_validator | ||
|
|
||
| from redisvl.schema.fields import BaseField | ||
| from redisvl.schema.schema import IndexInfo, IndexSchema | ||
|
|
||
| _ENV_PATTERN = re.compile(r"\$\{([^}:]+)(?::-([^}]*))?\}") | ||
|
|
||
|
|
||
| class MCPRuntimeConfig(BaseModel): | ||
| """Runtime limits and validated field mappings for MCP requests.""" | ||
|
|
||
| index_mode: str = "create_if_missing" | ||
| text_field_name: str | ||
| vector_field_name: str | ||
| default_embed_field: str | ||
| default_limit: int = 10 | ||
| max_limit: int = 100 | ||
| max_upsert_records: int = 64 | ||
| skip_embedding_if_present: bool = True | ||
| startup_timeout_seconds: int = 30 | ||
| request_timeout_seconds: int = 60 | ||
| max_concurrency: int = 16 | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_limits(self) -> "MCPRuntimeConfig": | ||
| if self.index_mode not in {"validate_only", "create_if_missing"}: | ||
| raise ValueError( | ||
| "runtime.index_mode must be validate_only or create_if_missing" | ||
| ) | ||
| if self.default_limit <= 0: | ||
| raise ValueError("runtime.default_limit must be greater than 0") | ||
| if self.max_limit < self.default_limit: | ||
| raise ValueError( | ||
| "runtime.max_limit must be greater than or equal to runtime.default_limit" | ||
| ) | ||
| if self.max_upsert_records <= 0: | ||
| raise ValueError("runtime.max_upsert_records must be greater than 0") | ||
| if self.startup_timeout_seconds <= 0: | ||
| raise ValueError("runtime.startup_timeout_seconds must be greater than 0") | ||
| if self.request_timeout_seconds <= 0: | ||
| raise ValueError("runtime.request_timeout_seconds must be greater than 0") | ||
| if self.max_concurrency <= 0: | ||
| raise ValueError("runtime.max_concurrency must be greater than 0") | ||
| return self | ||
|
|
||
|
|
||
| class MCPVectorizerConfig(BaseModel): | ||
| """Vectorizer constructor contract loaded from YAML.""" | ||
|
|
||
| model_config = ConfigDict(populate_by_name=True, extra="allow") | ||
|
|
||
| class_name: str = Field(alias="class", min_length=1) | ||
| model: str = Field(..., min_length=1) | ||
|
|
||
| @property | ||
| def extra_kwargs(self) -> Dict[str, Any]: | ||
| """Return vectorizer kwargs other than the normalized `class` and `model`.""" | ||
| return dict(self.model_extra or {}) | ||
|
|
||
| def to_init_kwargs(self) -> Dict[str, Any]: | ||
| """Build kwargs suitable for directly instantiating the vectorizer.""" | ||
| return {"model": self.model, **self.extra_kwargs} | ||
|
|
||
|
|
||
| class MCPConfig(BaseModel): | ||
| """Validated MCP server configuration loaded from YAML.""" | ||
|
|
||
| redis_url: str = Field(..., min_length=1) | ||
| index: IndexInfo | ||
| fields: Union[List[Dict[str, Any]], Dict[str, Dict[str, Any]]] | ||
| vectorizer: MCPVectorizerConfig | ||
| runtime: MCPRuntimeConfig | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_runtime_mapping(self) -> "MCPConfig": | ||
| """Ensure runtime field mappings point at explicit schema fields.""" | ||
| schema = self.to_index_schema() | ||
| field_names = set(schema.field_names) | ||
|
|
||
| if self.runtime.text_field_name not in field_names: | ||
| raise ValueError( | ||
| f"runtime.text_field_name '{self.runtime.text_field_name}' not found in schema" | ||
| ) | ||
|
|
||
| if self.runtime.default_embed_field not in field_names: | ||
| raise ValueError( | ||
| f"runtime.default_embed_field '{self.runtime.default_embed_field}' not found in schema" | ||
| ) | ||
|
|
||
| vector_field = schema.fields.get(self.runtime.vector_field_name) | ||
| if vector_field is None: | ||
| raise ValueError( | ||
| f"runtime.vector_field_name '{self.runtime.vector_field_name}' not found in schema" | ||
| ) | ||
| if vector_field.type != "vector": | ||
| raise ValueError( | ||
| f"runtime.vector_field_name '{self.runtime.vector_field_name}' must reference a vector field" | ||
| ) | ||
|
|
||
| return self | ||
|
|
||
| def to_index_schema(self) -> IndexSchema: | ||
| """Convert the MCP config schema fragment into a reusable `IndexSchema`.""" | ||
| return IndexSchema.model_validate( | ||
| { | ||
| "index": self.index.model_dump(mode="python"), | ||
| "fields": self.fields, | ||
| } | ||
| ) | ||
|
|
||
| @property | ||
| def vector_field(self) -> BaseField: | ||
| """Return the configured vector field from the generated index schema.""" | ||
| return self.to_index_schema().fields[self.runtime.vector_field_name] | ||
|
|
||
| @property | ||
| def vector_field_dims(self) -> Optional[int]: | ||
| """Return the configured vector dimension when the field exposes one.""" | ||
| attrs = self.vector_field.attrs | ||
| return getattr(attrs, "dims", None) | ||
|
|
||
|
|
||
| def _substitute_env(value: Any) -> Any: | ||
| """Recursively resolve `${VAR}` and `${VAR:-default}` placeholders.""" | ||
| if isinstance(value, dict): | ||
| return {key: _substitute_env(item) for key, item in value.items()} | ||
| if isinstance(value, list): | ||
| return [_substitute_env(item) for item in value] | ||
| if not isinstance(value, str): | ||
| return value | ||
|
|
||
| def replace(match: re.Match[str]) -> str: | ||
| name = match.group(1) | ||
| default = match.group(2) | ||
| env_value = os.environ.get(name) | ||
| if env_value is not None: | ||
| return env_value | ||
| if default is not None: | ||
| return default | ||
| # Fail fast here so startup never proceeds with partially-resolved config. | ||
| raise ValueError(f"Missing required environment variable: {name}") | ||
|
|
||
| return _ENV_PATTERN.sub(replace, value) | ||
|
|
||
|
|
||
| def load_mcp_config(path: str) -> MCPConfig: | ||
| """Load, substitute, and validate the MCP YAML configuration file.""" | ||
| config_path = Path(path).expanduser() | ||
| if not config_path.exists(): | ||
| raise FileNotFoundError(f"MCP config file {path} does not exist") | ||
|
|
||
| try: | ||
| with config_path.open("r", encoding="utf-8") as file: | ||
| raw_data = yaml.safe_load(file) | ||
| except yaml.YAMLError as exc: | ||
| raise ValueError(f"Invalid MCP config YAML: {exc}") from exc | ||
|
|
||
| if not isinstance(raw_data, dict): | ||
| raise ValueError("Invalid MCP config YAML: root document must be a mapping") | ||
|
|
||
| substituted = _substitute_env(raw_data) | ||
| return MCPConfig.model_validate(substituted) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import asyncio | ||
| from enum import Enum | ||
| from typing import Any, Dict, Optional | ||
|
|
||
| from pydantic import ValidationError | ||
| from redis.exceptions import RedisError | ||
|
|
||
| from redisvl.exceptions import RedisSearchError | ||
|
|
||
|
|
||
| class MCPErrorCode(str, Enum): | ||
| """Stable internal error codes exposed by the MCP framework.""" | ||
|
|
||
| INVALID_REQUEST = "invalid_request" | ||
| DEPENDENCY_MISSING = "dependency_missing" | ||
| BACKEND_UNAVAILABLE = "backend_unavailable" | ||
| INTERNAL_ERROR = "internal_error" | ||
|
|
||
|
|
||
| class RedisVLMCPError(Exception): | ||
| """Framework-facing exception carrying a stable MCP error contract.""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| message: str, | ||
| *, | ||
| code: MCPErrorCode, | ||
| retryable: bool, | ||
| metadata: Optional[Dict[str, Any]] = None, | ||
| ) -> None: | ||
| super().__init__(message) | ||
| self.code = code | ||
| self.retryable = retryable | ||
| self.metadata = metadata or {} | ||
|
|
||
|
|
||
| def map_exception(exc: Exception) -> RedisVLMCPError: | ||
| """Map framework exceptions into deterministic MCP-facing exceptions.""" | ||
| if isinstance(exc, RedisVLMCPError): | ||
| return exc | ||
|
|
||
| if isinstance(exc, (ValidationError, ValueError, FileNotFoundError)): | ||
| return RedisVLMCPError( | ||
| str(exc), | ||
| code=MCPErrorCode.INVALID_REQUEST, | ||
| retryable=False, | ||
| ) | ||
|
|
||
| if isinstance(exc, ImportError): | ||
| return RedisVLMCPError( | ||
| str(exc), | ||
| code=MCPErrorCode.DEPENDENCY_MISSING, | ||
| retryable=False, | ||
| ) | ||
|
|
||
| if isinstance( | ||
| exc, (TimeoutError, asyncio.TimeoutError, RedisSearchError, RedisError) | ||
| ): | ||
| return RedisVLMCPError( | ||
| str(exc), | ||
| code=MCPErrorCode.BACKEND_UNAVAILABLE, | ||
| retryable=True, | ||
| ) | ||
|
|
||
| return RedisVLMCPError( | ||
| str(exc), | ||
| code=MCPErrorCode.INTERNAL_ERROR, | ||
| retryable=False, | ||
| ) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeated schema reconstruction on every property access
Low Severity
The
vector_fieldandvector_field_dimsproperties each callto_index_schema(), which fully reconstructs and re-validates anIndexSchemaviamodel_validateon every access. The model validator_validate_runtime_mappingalso callsto_index_schema(). Any code path accessingvector_field_dims(e.g.,_validate_vectorizer_dimsduring startup) triggers two redundant schema constructions. Caching the result or computing these values once during validation would avoid the repeated work.Additional Locations (1)
redisvl/mcp/config.py#L107-L115