feat(mcp): let operators disable individual built-in tools (#668)

`list-indexes`, `search-records`, and `upsert-records` all registered
unconditionally, so an operator who wanted a narrower tool surface had
no way to get one. A deployment that should never advertise writes still
published `upsert-records` whenever any binding was writable, and a
single-purpose server still published discovery.

## The config

```yaml
server:
  builtin_tools:
    upsert-records: disabled
```

Omitted names stay enabled, so **existing configs are unaffected**. Only
the three real names are accepted: `search_records` with underscores
fails at startup rather than silently disabling nothing while reading as
though it had.

## Two unusable-but-valid tool surfaces now warn

Both shapes are legal config that presents to a client as a server that
simply does not work, and both were previously silent:

- **Everything disabled** leaves a server that connects and offers
nothing. The warning deliberately does not name a cause, since
`upsert-records` can also be absent because every binding is read-only.
- **Discovery disabled on a multi-index server** leaves `search-records`
demanding a logical index id that clients have no way to learn — its own
description tells them to call `list-indexes` first.

Neither is fatal, because an operator may be mid-rollout.

The multi-index discovery case had **no coverage anywhere**, so this
adds it, along with the negative case: with a sole binding the `index`
argument defaults, so disabling discovery there is legitimate and stays
quiet. I verified by mutation that the new test fails when the warning
condition is neutered.

## Verification

- MCP unit tests: 237 passing
- `make check-types`: clean
- Pre-commit: passing

## Note on sequencing

This is independently useful and mergeable on its own. It is also a
prerequisite for the custom tool profiles work that follows, since a
profile name must be checked against the built-in names this PR
introduces.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes which MCP tools clients see and discovery/write advertising;
misconfiguration can yield empty or hard-to-use tool surfaces, though
defaults preserve existing behavior and validation plus warnings reduce
silent failures.
> 
> **Overview**
> Adds **`server.builtin_tools`** so operators can disable any of
`list-indexes`, `search-records`, or `upsert-records` individually
(omitted tools stay enabled). Unknown names fail at **startup**
validation instead of being ignored.
> 
> **Registration** gates each built-in on config; **`list-indexes`**
sets **`upsert_available`** to false when upsert is disabled even if
bindings are writable. On **multi-index** servers with discovery off,
**search** and **upsert** tool descriptions **inline** logical index
ids. **Startup warnings** cover empty tool lists, discovery disabled
with multiple indexes, and **config changes** after tools were already
registered (process restart required to change the published surface).
> 
> Docs in **`docs/concepts/mcp.md`** describe the config and behavior.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
f323df5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: vishal-bala

docs/concepts/mcp.md

@@ -86,7 +86,7 @@ MCP-reserved score metadata field names for the configured search mode.
 
 ## Read-Only and Read-Write Modes
 
-RedisVL MCP always registers `search-records` and `list-indexes`.
+RedisVL MCP registers `search-records` and `list-indexes` by default (see [Tool Surface](#tool-surface) for turning a built-in off deliberately).
 
 Write availability is enforced at two levels:
 
@@ -105,12 +105,31 @@ For configuration and the gateway boundary, see {doc}`/user_guide/how_to_guides/
 
 ## Tool Surface
 
-RedisVL MCP exposes up to three tools:
+RedisVL MCP exposes up to three built-in tools:
 
-- `list-indexes` enumerates the configured logical indexes for discovery (always available)
+- `list-indexes` enumerates the configured logical indexes for discovery
 - `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
 
+Any of the three can be turned off with `server.builtin_tools` — useful for a server that should only ever read, or one that should not advertise discovery:
+
+```yaml
+server:
+  builtin_tools:
+    upsert-records: disabled
+```
+
+Only the three names above are accepted; anything else fails at startup rather than being silently ignored.
+
+Disabling a built-in adjusts what the rest of the surface advertises, so the published contract never points at something the server withholds:
+
+- `list-indexes` reports `upsert_available: false` for every binding when `upsert-records` is disabled, since a writable binding still cannot be written to through a tool that is not published.
+- On a multi-index server with `list-indexes` disabled, every tool that requires an `index` — `search-records` and `upsert-records` alike — names the available index ids in its own description instead of deferring to a discovery tool that does not exist. That server still logs a startup warning naming the affected tools, because inlining the ids is a fallback rather than an endorsement of the shape.
+
+A server whose tool set ends up unusable — no tools at all, or discovery disabled on a multi-index server — logs a warning at startup.
+
+Tools register once per process. `builtin_tools` is re-read on restart, but the registered tool set is not rebuilt, so a stop/start against an edited config keeps the previous tools and logs a warning saying so. Start a new process to change the tool surface.
+
 These tools follow a stable contract:
 
 - request validation happens before query or write execution

redisvl/mcp/config.py

@@ -26,11 +26,23 @@
 )
 
 
+_BUILTIN_TOOL_NAMES = frozenset({"list-indexes", "search-records", "upsert-records"})
+
+
 def reserved_score_metadata_field_names() -> frozenset[str]:
     """Return MCP-reserved score metadata field names."""
     return _RESERVED_SCORE_METADATA_FIELDS
 
 
+def builtin_tool_names() -> frozenset[str]:
+    """Return the names of the built-in MCP tools.
+
+    These register by default and can be turned off individually through
+    ``server.builtin_tools``, so they are not unconditionally available.
+    """
+    return _BUILTIN_TOOL_NAMES
+
+
 class MCPRuntimeConfig(BaseModel):
     """Runtime limits and validated field mappings for MCP requests."""
 
@@ -200,6 +212,25 @@ class MCPServerConfig(BaseModel):
     redis_url: str = Field(..., min_length=1)
     auth: MCPAuthConfig | None = None
     transport_security: MCPTransportSecurityConfig | None = None
+    builtin_tools: dict[str, Literal["enabled", "disabled"]] = Field(
+        default_factory=dict
+    )
+
+    @model_validator(mode="after")
+    def _validate_builtin_tools(self) -> "MCPServerConfig":
+        """Reject disable/enable entries that do not name a built-in tool."""
+        unknown = sorted(set(self.builtin_tools) - builtin_tool_names())
+        if unknown:
+            raise ValueError(
+                "server.builtin_tools contains unknown tool names: "
+                f"{', '.join(unknown)}; known built-ins: "
+                f"{', '.join(sorted(builtin_tool_names()))}"
+            )
+        return self
+
+    def builtin_tool_enabled(self, tool_name: str) -> bool:
+        """Report whether a built-in tool should be registered."""
+        return self.builtin_tools.get(tool_name, "enabled") == "enabled"
 
 
 class MCPIndexSearchConfig(BaseModel):

redisvl/mcp/server.py

@@ -68,6 +68,7 @@ def __init__(self, settings: MCPSettings):
         self._bindings: dict[str, BindingRuntime] = {}
         self._semaphore: asyncio.Semaphore | None = None
         self._tools_registered = False
+        self._registered_tool_fingerprint = ""
 
         # Lifecycle management
         self._lifecycle_state = _LifecycleState.INITIAL  # Server lifecycle
@@ -270,9 +271,32 @@ async def _probe_native_hybrid_search(index: AsyncSearchIndex) -> bool:
 
         return hasattr(client.ft(index.schema.index.name), "hybrid_search")
 
+    @staticmethod
+    def _tool_surface_fingerprint(config: Any) -> str:
+        """Summarize the config that a registered tool set baked in."""
+        if config is None:
+            return ""
+        return repr(sorted(config.server.builtin_tools.items()))
+
     def _register_tools(self) -> None:
         """Register MCP tools once every binding is ready."""
         if self._tools_registered or not hasattr(self, "tool"):
+            # Registration is deliberately once-per-process, since re-registering
+            # the same names on the FastMCP object is not valid. Built-in tool
+            # closures resolve their binding per call, so they survive a restart
+            # unchanged -- but which built-ins exist is now a function of config,
+            # and `startup()` re-reads that file. A stop/start against an edited
+            # config therefore keeps the old tool set, and the dangerous direction
+            # is an operator disabling a tool and believing the restart applied it.
+            if self._tools_registered:
+                current = self._tool_surface_fingerprint(getattr(self, "config", None))
+                if current != self._registered_tool_fingerprint:
+                    logger.warning(
+                        "MCP built-in tool configuration changed since tools were "
+                        "registered, but tools register once per process. The "
+                        "previously registered tool set is still in effect; "
+                        "restart the process to apply the new configuration."
+                    )
             return
 
         # The search description advertises schema-specific filter hints, which
@@ -282,17 +306,90 @@ def _register_tools(self) -> None:
         if len(self._bindings) == 1:
             search_schema = next(iter(self._bindings.values())).schema
 
-        # Discovery is always available so clients can enumerate indexes.
-        register_list_indexes_tool(self)
-        register_search_tool(self, search_schema)
+        # An operator can turn off a built-in whose capability the server should
+        # not offer at all -- a read-only deployment, or one that should not
+        # advertise discovery.
+        config = getattr(self, "config", None)
+        enabled = (
+            config.server.builtin_tool_enabled
+            if config is not None
+            else lambda _name: True
+        )
+
+        registered: list[str] = []
+
+        # Discovery is on by default so clients can enumerate indexes.
+        discovery_enabled = enabled("list-indexes")
+        if discovery_enabled:
+            register_list_indexes_tool(self)
+            registered.append("list-indexes")
+
+        # `index` is required once several bindings exist, and without discovery
+        # the logical ids cannot be learned any other way -- so every tool that
+        # requires one has to name them inline instead of deferring to a tool that
+        # is not published. Computed once so the two cannot drift apart.
+        unlisted_index_ids = (
+            sorted(self._bindings)
+            if len(self._bindings) > 1 and not discovery_enabled
+            else None
+        )
+
+        if enabled("search-records"):
+            register_search_tool(self, search_schema, index_ids=unlisted_index_ids)
+            registered.append("search-records")
         # 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)
+        if enabled("upsert-records") and any(
+            not rt.effective_read_only for rt in self._bindings.values()
+        ):
+            register_upsert_tool(self, index_ids=unlisted_index_ids)
+            registered.append("upsert-records")
+
+        self._warn_on_unusable_tool_surface(registered)
+        self._registered_tool_fingerprint = self._tool_surface_fingerprint(config)
         self._tools_registered = True
 
+    def _warn_on_unusable_tool_surface(self, registered: list[str]) -> None:
+        """Warn about tool-set shapes that are valid config but unusable in practice.
+
+        Neither case is fatal -- an operator may be mid-rollout -- but both are
+        silent otherwise, and both present to a client as a server that simply
+        does not work.
+        """
+        if not registered:
+            # Deliberately does not attribute a cause: `upsert-records` can also
+            # be absent because every binding is read-only, not because
+            # `builtin_tools` disabled it.
+            logger.warning(
+                "MCP server registered no tools, so clients will see an empty "
+                "tool list. Check server.builtin_tools and read-only settings."
+            )
+            return
+
+        # Both `search-records` and `upsert-records` require an `index` once
+        # several bindings exist, so either one is affected by losing discovery --
+        # naming them in the descriptions keeps the contract satisfiable, but an
+        # operator who disabled discovery on a multi-index server probably did not
+        # intend to. Checking only search would leave a write-only surface silent.
+        index_requiring = sorted(
+            {"search-records", "upsert-records"}.intersection(registered)
+        )
+        if (
+            len(self._bindings) > 1
+            and index_requiring
+            and "list-indexes" not in registered
+        ):
+            logger.warning(
+                "MCP server has %d indexes and exposes %s, but list-indexes is "
+                "disabled: clients cannot discover the logical index ids those "
+                "tools require, so the ids are named inline in each tool "
+                "description instead.",
+                len(self._bindings),
+                ", ".join(index_requiring),
+            )
+
     @asynccontextmanager
     async def _server_lifespan(self, _server: Any):
         """Bridge FastMCP lifespan hooks onto the server's explicit lifecycle."""

redisvl/mcp/tools/list_indexes.py

@@ -49,13 +49,20 @@ def _binding_limits(binding_runtime: BindingRuntime) -> dict[str, int]:
     }
 
 
-def _describe_binding(binding_runtime: BindingRuntime) -> dict[str, Any]:
+def _describe_binding(
+    binding_runtime: BindingRuntime, *, upsert_tool_available: bool = True
+) -> 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
+    # Reflects global read-only, the per-index read_only policy, and whether the
+    # tool is published at all. A writable binding on a server that disabled
+    # `upsert-records` still cannot be written to, so reporting availability from
+    # read-only state alone would advertise a tool the client cannot call.
+    entry["upsert_available"] = (
+        upsert_tool_available and not binding_runtime.effective_read_only
+    )
     entry["fields"] = _binding_fields(binding_runtime)
     limits = _binding_limits(binding_runtime)
     if limits:
@@ -73,16 +80,26 @@ def list_indexes(server: "RedisVLMCPServer") -> dict[str, Any]:
     # client could misread as "no indexes configured".
     if not server._bindings:
         raise RuntimeError("MCP server has not been started")
+    config = getattr(server, "config", None)
+    upsert_tool_available = config is None or config.server.builtin_tool_enabled(
+        "upsert-records"
+    )
     return {
         "indexes": [
-            _describe_binding(binding_runtime)
+            _describe_binding(
+                binding_runtime, upsert_tool_available=upsert_tool_available
+            )
             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."""
+    """Register the read-only `list-indexes` MCP tool.
+
+    Registered by default; an operator can turn it off through
+    ``server.builtin_tools``.
+    """
 
     async def list_indexes_tool():
         """FastMCP wrapper for the `list-indexes` tool."""

redisvl/mcp/tools/search.py

@@ -51,16 +51,31 @@ def _build_return_fields_hint(schema: IndexSchema) -> str:
 
 
 def _build_search_tool_description(
-    schema: IndexSchema | None, base_description: str | None = None
+    schema: IndexSchema | None,
+    base_description: str | None = None,
+    *,
+    index_ids: list[str] | None = None,
 ) -> str:
     """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.
+    an index per call), so per-field hints are omitted and a routing note is
+    appended instead.
+
+    ``index_ids`` is supplied only when discovery is unavailable -- an operator
+    can disable ``list-indexes``, and pointing clients at a tool the server does
+    not publish would leave them unable to satisfy the required ``index``
+    argument at all. Naming the ids inline is the only way they can learn them.
     """
     description = (base_description or DEFAULT_SEARCH_DESCRIPTION).strip()
     if schema is None:
+        if index_ids:
+            return (
+                description + " Multiple indexes are configured and discovery is "
+                "disabled: pass one of these index ids as the `index` argument: "
+                + ", ".join(index_ids)
+                + "."
+            )
         return (
             description + " Multiple indexes are configured: call list-indexes "
             "first, then pass the chosen index id as the `index` argument."
@@ -498,9 +513,12 @@ async def search_records(
         raise map_exception(exc) from exc
 
 
-def register_search_tool(server: Any, schema: IndexSchema | None) -> None:
+def register_search_tool(
+    server: Any, schema: IndexSchema | None, *, index_ids: list[str] | None = None
+) -> None:
     """Register the MCP `search-records` tool with its config-owned contract."""
     description = _build_search_tool_description(
+        index_ids=index_ids,
         schema=schema,
         base_description=server.mcp_settings.tool_search_description,
     )

redisvl/mcp/tools/upsert.py

@@ -360,11 +360,24 @@ async def upsert_records(
         raise map_exception(exc) from exc
 
 
-def register_upsert_tool(server: Any) -> None:
-    """Register the MCP upsert tool on a server-like object."""
+def register_upsert_tool(server: Any, *, index_ids: list[str] | None = None) -> None:
+    """Register the MCP upsert tool on a server-like object.
+
+    ``index_ids`` is supplied only when discovery is unavailable on a multi-index
+    server. ``index`` is required there, and with ``list-indexes`` withheld the
+    logical ids cannot be learned any other way, so naming them inline is what
+    keeps the published contract satisfiable.
+    """
     description = (
         server.mcp_settings.tool_upsert_description or DEFAULT_UPSERT_DESCRIPTION
     )
+    if index_ids:
+        description = (
+            description.strip() + " Multiple indexes are configured and discovery "
+            "is disabled: pass one of these index ids as the `index` argument: "
+            + ", ".join(index_ids)
+            + "."
+        )
 
     async def upsert_records_tool(
         records: list[dict[str, Any]],

tests/integration/test_mcp/test_upsert_tool.py

@@ -491,7 +491,7 @@ async def test_read_only_mode_excludes_upsert_tool(
     )
     monkeypatch.setattr(
         "redisvl.mcp.server.register_search_tool",
-        lambda server, schema: None,
+        lambda server, schema, index_ids=None: None,
     )
 
     def fake_tool(*args: Any, **kwargs: Any):
@@ -506,7 +506,7 @@ def decorator(func: Any) -> Any:
 
     called: list[bool] = []
 
-    def fake_register_upsert_tool(server: Any) -> None:
+    def fake_register_upsert_tool(server: Any, index_ids: Any = None) -> None:
         called.append(server.mcp_settings.read_only)
 
     monkeypatch.setattr(