Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion redisvl/mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ def _register_tools(self) -> None:
# Discovery is always available so clients can enumerate indexes.
register_list_indexes_tool(self)
register_search_tool(self, search_schema)
if not self.mcp_settings.read_only:
# Expose upsert only when at least one binding is writable. A binding is
# read-only under global read-only mode or its own read_only policy, both
# of which are folded into effective_read_only; the per-call write check
# in the tool then rejects writes to any individual read-only binding.
if any(not rt.effective_read_only for rt in self._bindings.values()):
register_upsert_tool(self)
self._tools_registered = True

Expand Down
27 changes: 22 additions & 5 deletions redisvl/mcp/tools/upsert.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,13 +249,27 @@ async def upsert_records(
server: Any,
*,
records: list[dict[str, Any]],
index: str | None = None,
id_field: str | None = None,
skip_embedding_if_present: bool | None = None,
) -> dict[str, Any]:
"""Execute `upsert-records` against the selected Redis index binding."""
"""Execute `upsert-records` against the selected Redis index binding.

``index`` names the logical binding to write to. It is optional when exactly
one binding is configured and required when multiple exist. Writes to a
read-only binding (whether from global read-only mode or the binding's own
``read_only`` policy) are rejected with ``invalid_request``. The resolved
logical id is echoed back in the response.
"""
try:
rt = server.resolve_binding(None)
index = rt.index
rt = server.resolve_binding(index)
if rt.effective_read_only:
raise RedisVLMCPError(
f"index '{rt.binding_id}' is read-only",
code=MCPErrorCode.INVALID_REQUEST,
retryable=False,
)
index_obj = rt.index
runtime = rt.binding.runtime
effective_skip_embedding = _validate_request(
runtime=runtime,
Expand All @@ -269,7 +283,7 @@ async def upsert_records(
for record in prepared_records:
_validate_record(
record,
index=index,
index=index_obj,
vector_field_name=runtime.vector_field_name,
)
if rt.binding.supports_server_side_embedding:
Expand Down Expand Up @@ -326,7 +340,7 @@ async def upsert_records(
try:
keys = await server.run_guarded(
"upsert-records",
index.load(loadable_records, id_field=id_field),
index_obj.load(loadable_records, id_field=id_field),
timeout_seconds=runtime.request_timeout_seconds,
)
except Exception as exc:
Expand All @@ -335,6 +349,7 @@ async def upsert_records(
raise mapped from exc

return {
"index": rt.binding_id,
"status": "success",
"keys_upserted": len(keys),
"keys": keys,
Expand All @@ -353,6 +368,7 @@ def register_upsert_tool(server: Any) -> None:

async def upsert_records_tool(
records: list[dict[str, Any]],
index: str | None = None,
id_field: str | None = None,
skip_embedding_if_present: bool | None = None,
):
Expand All @@ -362,6 +378,7 @@ async def upsert_records_tool(
return await upsert_records(
server,
records=records,
index=index,
id_field=id_field,
skip_embedding_if_present=skip_embedding_if_present,
)
Expand Down
124 changes: 124 additions & 0 deletions tests/integration/test_mcp/test_upsert_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,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.INVALID_REQUEST


@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
Expand Down
61 changes: 59 additions & 2 deletions tests/unit/test_mcp/test_server_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,17 @@ async def test_probe_native_hybrid_search_false_for_old_redis_py(monkeypatch):
assert client.info_calls == 0


def _binding_runtime(binding_id: str) -> BindingRuntime:
def _binding_runtime(
binding_id: str, *, effective_read_only: bool = False
) -> BindingRuntime:
return BindingRuntime(
binding_id=binding_id,
binding=SimpleNamespace(),
index=SimpleNamespace(),
schema=SimpleNamespace(),
vectorizer=None,
supports_native_hybrid_search=False,
effective_read_only=False,
effective_read_only=effective_read_only,
)


Expand Down Expand Up @@ -110,3 +112,58 @@ def test_resolve_binding_rejects_unknown_index():

assert excinfo.value.code == MCPErrorCode.INVALID_REQUEST
assert "missing" in str(excinfo.value)


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
Loading
Loading