feat(mcp): support multiple active index bindings (RAAE-1604) - #629
Conversation
🛡️ Jit Security Scan Results✅ No security findings were detected in this PR
Security scan by Jit
|
|
CI is failing because of flaky tests that will be fixed by redis-developer/sql-redis#39 |
There was a problem hiding this comment.
I really like the design!
Binding runtime dataclass is nice and like how resolve binding works as the single routing entry point shared across all tools keeps the behavior consistent and how it's backward compatible for single binding servers works
One blocking issue before merge:
the teardown loop in _teardown_runtime does not protect subsequent bindings if _close_resources raises on an earlier one.
- A failed disconnect on binding 1 will propagate out of the loop immediately, leaving bindings 2 through N with open Redis connections that are never closed.
- In a partial startup failure this is exactly the path that gets exercised, so the leak is not hypothetical.
- Wrapping each iteration in a try/except that logs and continues is a small fix and should be done before this lands.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3e441b2. Configure here.
nkanu17
left a comment
There was a problem hiding this comment.
Thank you for the changes! Looks good! 🚀
Treat the MCP `indexes` config as a real multi-binding map instead of enforcing exactly one logical binding. This is the foundation for multi-index MCP support (RAAE-1603); single-index configs and callers behave identically. Config: - Drop the "exactly one configured index binding" validator; require at least one binding with non-blank ids. - Add per-binding `description` and `read_only` fields. - Move schema inspection / runtime-mapping / search validation methods onto MCPIndexBindingConfig and remove the single-binding convenience accessors from MCPConfig. Runtime/server: - Introduce an immutable BindingRuntime bundling a binding's config, index, effective schema, vectorizer, native-hybrid support, and effective read-only flag. - Replace single-resource server state with a dict of BindingRuntime; startup inspects/validates/initializes every binding independently (one client per binding) with all-or-nothing teardown. - Resolve native-hybrid support eagerly per binding. - Size the concurrency semaphore from the max binding limit and pass a per-binding request timeout into run_guarded. - Add resolve_binding(index_id): defaults to the sole binding, raises invalid_request for omitted-on-multi and unknown ids. - Compute effective_read_only as global read-only OR per-binding read_only. Tools: - Re-thread search/upsert onto a resolved BindingRuntime instead of single-binding server accessors. Tests follow TDD: multi-binding config/startup coverage, resolve_binding semantics, semaphore sizing, per-binding teardown, and backward-compatible single-index behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three issues found in review: - upsert_records() never consulted rt.effective_read_only, so a binding configured with read_only: true could still accept writes when the global read_only flag was off. Add an explicit FORBIDDEN guard at the top of upsert_records() before any other processing. - _teardown_runtime() did not protect subsequent bindings if _close_resources() raised on an earlier one. A network error on binding 1 would propagate out of the loop, leaving bindings 2-N with open Redis connections. Wrap each iteration in try/except and log the failure so teardown is best-effort across all bindings. - _tools_registered was not reset in _teardown_runtime(), so a re-startup after a partial-startup failure would skip tool re-registration. Reset it alongside the other state clears. Also adds a clarifying comment on the semaphore size semantics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add regression tests for the behaviors introduced in 3a0d1fe, which shipped without coverage: - upsert-records rejects a write to a read-only binding with FORBIDDEN before any backend load is attempted. - _teardown_runtime continues closing the remaining bindings when one binding's close raises, and fully clears terminal state (including _tools_registered) so a later startup re-registers tools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both convenience methods called resolve_binding(None), which raises invalid_request on any multi-index server — a latent footgun for tool authors that only surfaced at runtime under a multi-binding config. They had no non-test callers. Remove them and route all access through resolve_binding. Integration tests that used them now use small mcp_binding_index / mcp_binding_vectorizer test helpers in tests/conftest.py, which delegate to resolve_binding and preserve the same "vectorizer is not configured" / "has not been started" errors. Drop the now-dead mirror methods from the unit test fakes. Also align test_server_shutdown_disconnects_index_when_vectorizer_close_fails with the best-effort teardown introduced in 3a0d1fe: a failing vectorizer close is logged and swallowed (index still disconnected), so shutdown no longer re-raises it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1604) Stop resetting _tools_registered in _teardown_runtime. MCP tools are registered once on the FastMCP instance and their closures resolve the live binding at call time, so they survive teardown and stay valid across a stop/start. The lifecycle allows startup() again from STOPPED, and _register_tools() runs inside _initialize_runtime_resources, so resetting the flag caused a restart to re-register the same search-records/upsert-records names on the instance (duplicate/broken restart). The earlier "reset so a failed retry re-registers" concern does not occur: the flag is only set True after registration succeeds, and registrations persist on the instance regardless of binding teardown, so a retry that skips registration still has working tools. Update the teardown test to assert registration survives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fulltext/hybrid MCP configs that omit `stopwords` inherit RedisVL's TextQuery default of "english", which lazily loads the nltk stopwords corpus (and attempts a network `nltk.download` at query time). That made auth/routing tests transitively depend on the nltk corpus being present on the runner — an unrelated, flaky failure mode (e.g. test_http_transport_enforces_jwt_auth failing on a runner missing corpora/stopwords/english). None of these tests assert stopword behavior; fulltext is only a vehicle for exercising auth, routing, startup, and upsert. Set `stopwords: None` explicitly on every such config so they no longer touch nltk. Tests that genuinely cover search behavior already set `stopwords: None`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-1604) Read read_scope/write_scope directly off the typed MCPAuthConfig instead of a silent getattr(..., None). The old double-getattr would fail open — yielding None and skipping the scope check — if the field were ever renamed; direct access fails loud. Mirrors the same fix applied to list_indexes.py and keeps the three tool wrappers consistent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (RAAE-1604) Move binding teardown into startup()'s exception handler so it covers every post-begin failure, not just failures inside _initialize_runtime_resources. Previously, if initialization succeeded (clients connected, tools registered) and a later step such as _mark_running() raised, the handler marked the server stopped without tearing down, leaking Redis clients and vectorizers; a later shutdown() saw STOPPED and skipped cleanup. _initialize_runtime_resources no longer needs its own teardown (a binding that fails mid-build still closes its bare client inside _initialize_binding), giving a single fail-closed teardown path. Add a regression test for the init-succeeds-then-post-step-fails case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
118b01d to
bd2a28a
Compare
f80e98f
into
feature/raae-1603-mcp-multi-index
> Stacked on #629 (RAAE-1604). Review/merge that first; this PR targets the 1604 branch so the diff is scoped to the discovery tool. ## Motivation Once a single MCP server can expose multiple logical indexes ([RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)), clients need a lightweight way to discover what's available so they can pick the right index instead of guessing. This PR ([RAAE-1605](https://redislabs.atlassian.net/browse/RAAE-1605)) adds an always-registered, read-only `list-indexes` tool for exactly that, and it grounds discovery in the schema the server already inspected at startup rather than asking users to re-declare field metadata in config. ## Implementation The new tool (`redisvl/mcp/tools/list_indexes.py`) returns one entry per configured binding, in configured order: the logical `id`, an optional `description`, an `upsert_available` flag, the shared filterable `fields`, and — only when explicitly configured — a `limits` object. `upsert_available` is simply `not effective_read_only`, so it already reflects both the global `--read-only` flag and the per-index `read_only` policy resolved at startup. The `fields` list is built from the binding's effective (inspected + overridden) schema that already lives on its `BindingRuntime`, so the output stays consistent with what the index actually contains; the vector field and the configured default embed-source text field are omitted because they are implementation inputs rather than fields a client would filter on. The Redis index name (`redis_name`) is deliberately never exposed. Limits are included only when the operator set them explicitly — detected via the runtime model's `model_fields_set` — so defaults don't masquerade as deliberate overrides; per the contract this covers `max_limit` and `max_upsert_records`. The tool is registered unconditionally during the server's tool registration (alongside `search-records` and the conditionally-registered `upsert-records`) and is gated by the same read scope as search when auth is enabled, since it is read-only. - Output is deterministic and ordered by configured binding. - No new configuration surface or settings are required. ## Verification - `mypy` clean; `black`/`isort` formatted. - New unit coverage: field omission (vector + embed-source), description/limits inclusion rules, `redis_name` secrecy, read-only reflection, single- and multi-binding output, and tool registration. - New integration test starts a real two-binding server (one vector, one fulltext) and asserts the discovered fields come from the inspected schema and follow the omission rules. - Full MCP suite green: 178 unit + 45 integration (2 skipped on Redis-version gates) against Redis 8. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1605]: https://redislabs.atlassian.net/browse/RAAE-1605?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Read-only discovery metadata with no new config; behavior is additive and gated like existing search tools when auth is enabled. > > **Overview** > Adds a read-only **`list-indexes`** MCP tool so clients can discover logical indexes on multi-binding servers before calling **`search-records`** or **`upsert-records`**. > > The tool is **always registered** during `_register_tools` (alongside search; upsert remains conditional). Each binding is returned in config order with **`id`**, optional **`description`**, **`upsert_available`** (`not effective_read_only`), filterable **`fields`** from the startup-inspected schema (vector and default embed-source text omitted), and **`limits`** only when **`max_limit`** / **`max_upsert_records`** were explicitly set. **`redis_name`** is never exposed; empty bindings raise **`RuntimeError`** like other pre-startup paths. When auth is on, the tool uses the same **read scope** as search. > > Unit and integration tests cover payload rules, registration, and a two-binding startup scenario. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 45ce686. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Motivation With a single MCP server now able to expose multiple logical index bindings ([RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)) and advertise them via `list-indexes` ([RAAE-1605](https://redislabs.atlassian.net/browse/RAAE-1605)), the `search-records` tool still implicitly resolved the sole binding. On a multi-binding server it had no way to say *which* index a query should run against. This change ([RAAE-1606](https://redislabs.atlassian.net/browse/RAAE-1606)) makes that routing explicit while keeping the public contract backwards-safe for existing single-index callers. The design goal is that v1 clients see no behavioral change: when exactly one binding is configured and the caller omits `index`, the request resolves to that binding exactly as before. Routing only becomes mandatory once ambiguity exists. Query construction, validation, pagination, filtering, and the configured search mode all remain owned by the selected binding — this ticket adds only the selection and a confirmation echo, not new query behavior. ## Implemented changes `search-records` gains an optional `index` argument naming the logical binding to query. Resolution flows through the shared `resolve_binding` routing introduced in RAAE-1604, so the three cases fall out consistently: an omitted `index` with one binding resolves to that binding; an omitted `index` with multiple bindings returns `invalid_request`; and an unknown id returns `invalid_request`. The resolved logical id is echoed back as the `index` field of the response payload so multi-index clients can confirm where a query actually ran. All downstream work (limits, schema, vectorizer, search config) continues to read from the resolved binding's runtime. When multiple bindings are configured the tool description is ambiguous about fields, so instead of emitting per-field filter hints it now appends a short routing note directing the client to call `list-indexes` first and pass the chosen id as `index`. Single-binding servers keep their full schema-derived description unchanged. Minor additional changes: - The FastMCP wrapper exposes `index` as a tool parameter so MCP clients can supply it. - Unit coverage for default-to-sole-binding, named routing, unknown-id rejection, the wrapper param, and the ambiguous-schema description note. - Integration coverage for a two-binding server: routing to each named binding, omitted-index rejection, unknown-id rejection, and single-binding echo. ## Verification - `make format` (isort + black) and mypy clean on changed files. - Full MCP suite: **232 passed, 2 skipped** (Redis-version-gated) across unit + integration. ## Stacking This PR targets `feature/raae-1605-list-indexes` so its diff stays scoped to search routing. Review/merge bottom-up: [#629](#629) → [#630](#630) → this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1605]: https://redislabs.atlassian.net/browse/RAAE-1605?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1606]: https://redislabs.atlassian.net/browse/RAAE-1606?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Backward-compatible for single-index callers; changes are limited to MCP search routing and an additive response field, with validation delegated to existing `resolve_binding`. > > **Overview** > The MCP **`search-records`** tool now accepts an optional **`index`** argument so clients can target a specific logical binding on multi-index servers. Resolution goes through **`resolve_binding`**: omitting **`index`** still works when only one binding exists; with multiple bindings, **`index`** is required and unknown ids return **`invalid_request`**. > > Successful responses include a new **`index`** field with the resolved binding id so callers can confirm routing. The FastMCP wrapper exposes **`index`** as a tool parameter. > > When the server has multiple indexes, the tool description skips per-schema filter/return-field hints and instead tells clients to call **`list-indexes`** and pass the chosen id as **`index`**. Single-index servers keep the full schema-derived description. > > Unit and integration tests cover named routing, omitted-index errors on multi-binding setups, unknown-index rejection, single-binding echo behavior, and the ambiguous-schema description. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c2f30d5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ds (RAAE-1607) (#632) ## Motivation This completes the multi-index tool surface for the RedisVL MCP server. After [RAAE-1606](https://redislabs.atlassian.net/browse/RAAE-1606) taught `search-records` to route by logical index, `upsert-records` ([RAAE-1607](https://redislabs.atlassian.net/browse/RAAE-1607)) needs the same explicit routing — but writes also carry a policy dimension that reads do not. A single server can now host a mix of writable and read-only bindings, and the tool must respect both the global `--read-only` override and each binding's own `read_only` flag while staying backwards-safe for existing single-index clients. The design keeps single-index behavior identical: when one binding is configured and `index` is omitted, the write resolves to that binding exactly as before. Routing becomes mandatory only once multiple bindings exist. Write enforcement happens at two complementary levels so the contract is unambiguous: a server with no writable bindings should not advertise the tool at all, while a server with some writable bindings still needs to protect the read-only ones on a per-call basis. ## Implemented changes `upsert-records` gains an optional `index` argument naming the logical binding to write to, resolved through the shared `resolve_binding` routing introduced in RAAE-1604. An omitted `index` with one binding resolves to that binding; an omitted `index` with multiple bindings returns `invalid_request`; and an unknown id returns `invalid_request`. The resolved logical id is echoed back as the `index` field of the response, and the selected binding's embedding, runtime limits, and schema validation drive the rest of the write unchanged. Write availability is enforced at two levels. The tool registration gate is refined from "global read-only is off" to "at least one binding is writable" — expressed via `effective_read_only`, which already folds in both global read-only mode and a binding's own `read_only` policy — so an all-read-only server does not expose `upsert-records` at all. When the tool is registered, a per-call guard rejects writes to any individual read-only binding with `invalid_request` *before* any embedding or backend write occurs, so a writable server can still protect specific indexes. Minor additional changes: - The FastMCP wrapper exposes `index` as a tool parameter. - Unit coverage for default-to-sole-binding, named routing, unknown-id rejection, read-only-binding rejection, the wrapper param, and both registration-gate outcomes (any-writable exposes the tool; all-read-only hides it). - Integration coverage on a two-binding server (one writable vector index, one read-only fulltext index): routing to the writable binding, omitted-index rejection, unknown-id rejection, read-only-binding rejection, and single-binding echo. ## Verification - `make format` (isort + black) and mypy clean on changed files. - Full MCP suite: **244 passed, 2 skipped** (Redis-version-gated) across unit + integration. ## Stacking This PR targets `feature/raae-1606-search-routing` so its diff stays scoped to upsert routing + write policy. Review/merge bottom-up: [#629](#629) → [#630](#630) → [#631](#631) → this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1606]: https://redislabs.atlassian.net/browse/RAAE-1606?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1607]: https://redislabs.atlassian.net/browse/RAAE-1607?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes write routing and when the upsert tool is exposed on multi-index servers; mistakes could allow writes to the wrong index or hide/show the tool unexpectedly, though enforcement is fail-closed before backend writes. > > **Overview** > **`upsert-records`** now accepts an optional **`index`** logical binding id (via shared **`resolve_binding`**), matching multi-index **`search-records`**: omit **`index`** when one binding is configured; require it when several exist; reject unknown ids. Successful responses include an **`index`** field naming the binding that was written. > > Write policy is split between **tool advertisement** and **per-call enforcement**. **`upsert-records`** is registered only when at least one binding is writable (**`effective_read_only`** is false for some binding), not merely when global read-only mode is off. Each call still rejects writes to read-only bindings (**`FORBIDDEN`**, before embedding or Redis load) with a clearer message naming the binding. > > The FastMCP wrapper exposes **`index`** as a tool parameter. Unit and integration tests cover routing, registration gating, and read-only rejection. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 98aded7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Motivation The RedisVL MCP docs were written for the original single-index model and still stated that "one server process binds to exactly one existing Redis index." With multi-index support now implemented across [RAAE-1604](https://redislabs.atlassian.net/browse/RAAE-1604)–[RAAE-1607](https://redislabs.atlassian.net/browse/RAAE-1607), the user-facing documentation ([RAAE-1608](https://redislabs.atlassian.net/browse/RAAE-1608)) needs to describe the new compatibility story without confusing existing single-index users. The guiding principle in the rewrite is that single-index remains the simplest deployment and works exactly as before — callers never name an index — while multi-index is presented as a formal, additive capability layered on top via discovery (`list-indexes`) and explicit routing (the `index` argument). No documentation still claims a server must bind to exactly one index. ## Implemented changes The concept doc ([concepts/mcp.md](docs/concepts/mcp.md)) now frames the server as binding one *or several* logical indexes, each addressed by an id, and adds an "Index Selection and Discovery" section covering the optional `index` argument, the omitted-index and unknown-id rules, the response echo, and discovery-first guidance. The "Single Index Binding" section becomes "Single and Multiple Index Bindings," the read-only section explains the two-level write policy (global `--read-only` vs per-index `read_only`, folded into effective write availability), and the tool surface gains a `list-indexes` subsection documenting its minimal payload — filterable fields only, vector/embed-source fields omitted, explicit-only limits, and `redis_name` never exposed. The how-to guide ([how_to_guides/mcp.md](docs/user_guide/how_to_guides/mcp.md)) adds a two-binding config example (a writable vector index alongside a read-only fulltext index), an Index Selection subsection, a `list-indexes` tool contract with a response example, and threads the optional `index` argument through the `search-records` and `upsert-records` argument lists and request/response examples. A discovery-first multi-index flow is shown at the top of the search examples, and the CLI/env-var notes are updated for per-index read-only and the multi-index search description. Minor additional changes: - README MCP section and feature-table entry reworded from "an existing Redis index" to "one or more existing Redis indexes," with `list-indexes` and the discovery-first flow noted. (The README edit was explicitly authorized for this ticket, overriding the repo's default no-README-edits rule.) ## Verification - `sphinx-build` (the `docs` dependency group) completes cleanly (exit 0). The only warnings are pre-existing and unrelated (upstream `redis-py` docstrings and `index.md` heading levels); no warnings reference the MCP pages, and no broken cross-reference/anchor warnings were introduced. ## Stacking This PR targets `feature/raae-1607-upsert-routing` and is the top of the stack. Review/merge bottom-up: [#629](#629) → [#630](#630) → [#631](#631) → [#632](#632) → this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1604]: https://redislabs.atlassian.net/browse/RAAE-1604?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1607]: https://redislabs.atlassian.net/browse/RAAE-1607?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [RAAE-1608]: https://redislabs.atlassian.net/browse/RAAE-1608?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Markdown-only changes with no runtime, API, or security behavior modified. > > **Overview** > **Documentation-only** update aligning RedisVL MCP user-facing docs with multi-index support: one server can bind **one or several** logical indexes while **single-index deployments stay backward compatible** (omit `index`). > > **README** rewords MCP copy from a single index to one or more, and notes **`list-indexes`**, discovery-first routing, and per-index tools. > > **`docs/concepts/mcp.md`** reframes the model (per-index config, all-or-nothing startup), adds **Index Selection and Discovery** (`index` argument rules, response echo), expands **read-only** to global vs per-index `read_only` and effective `upsert_available`, and documents the **`list-indexes`** payload (minimal fields, no `redis_name`). > > **`docs/user_guide/how_to_guides/mcp.md`** adds a **two-binding YAML example**, **`list-indexes`** contract and examples, threads optional **`index`** through search/upsert docs, a discovery-first flow, and updates CLI/env notes for multi-index search tool descriptions. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit de2eb4f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Overview Rolls up the **RedisVL MCP multi-index server** epic ([RAAE-1603](https://redislabs.atlassian.net/browse/RAAE-1603)) into `main`. A single MCP server can now expose **one or more** configured Redis index bindings — with discovery (`list-indexes`), explicit per-call routing (an optional `index` argument on `search-records`/`upsert-records`), and per-index write policy — while existing **single-index configs and callers behave exactly as before**. ## This is a roll-up — every change was already reviewed and merged This PR introduces **no new code**. It is the aggregate of a stacked series of per-ticket PRs, each reviewed and squash-merged into the `feature/raae-1603-mcp-multi-index` collector branch: - [#629](#629) — RAAE-1604: config + runtime refactor (immutable `BindingRuntime`, per-binding startup/teardown, `resolve_binding` routing, single global concurrency semaphore) - [#630](#630) — RAAE-1605: `list-indexes` discovery tool - [#631](#631) — RAAE-1606: `search-records` index routing - [#632](#632) — RAAE-1607: `upsert-records` index routing + per-index write policy - [#633](#633) — RAAE-1608: concept/how-to/README docs Each stacked PR was reviewed (human + Cursor Bugbot) and its threads resolved before merge; this branch is the sum of those merges plus a sync with `main`. ## Synced with `main` The branch is up to date with `main`, including the recently merged MCP security work — **Host/Origin header validation ([#643](#643 and the **nltk drop ([#645](#645 — pulled in via merge, so this PR's diff contains **only** the epic's own changes (main's commits are in the merge base). ## Security review A security review of the epic diff was run (auth scope enforcement, read-only/write policy, index routing, info leakage, injection, config/vectorizer resolution). **No findings** — tokenless HTTP is rejected upstream by the SDK auth middleware, scopes and read-only policy are enforced at both registration and per-call, routing is confined to configured bindings, and `redis_name` is never exposed by `list-indexes`. ## Verification - Full MCP unit + integration suite: **283 passed / 1 skipped** on `redis:8.2` (now runs without nltk). - `mypy` clean; `black`/`isort` clean. - Backward compatibility covered by dedicated tests (single-binding defaults, omitted-`index` behavior, unchanged response contract aside from the additive `index` echo). 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1603]: https://redislabs.atlassian.net/browse/RAAE-1603?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes MCP server startup, routing, and write boundaries across bindings; single-index behavior is preserved but multi-index misconfiguration or routing errors could affect production assistants. > > **Overview** > **Multi-index MCP server** — One process can expose several logical index bindings from YAML `indexes`, each with its own schema inspection, search config, vectorizer, and runtime limits. Startup is all-or-nothing: any binding failure prevents the server from starting. > > **Routing and discovery** — New **`list-indexes`** tool enumerates logical ids, filterable fields, `upsert_available`, and optional limits (without exposing `redis_name`). **`search-records`** and **`upsert-records`** accept an optional **`index`** argument; responses echo the resolved id. With a single binding, omitting `index` still works; with multiple bindings, omitting or using an unknown id returns `invalid_request`. > > **Write policy** — Global `--read-only` / `REDISVL_MCP_READ_ONLY` plus per-binding **`read_only: true`** combine into **`effective_read_only`**. `upsert-records` is registered only if at least one binding is writable; writes to a read-only binding fail with **`forbidden`** before Redis is touched. > > **Implementation** — Config drops the “exactly one index” rule and adds **`description`** / **`read_only`** on bindings. Server state moves to per-binding **`BindingRuntime`** and **`resolve_binding()`**; removed single-index helpers like `get_index()`. Search tool descriptions omit per-schema hints when multiple indexes are configured and point clients to `list-indexes`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 553e55f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Overview Rolls up the **RedisVL MCP multi-index server** epic ([RAAE-1603](https://redislabs.atlassian.net/browse/RAAE-1603)) into `main`. A single MCP server can now expose **one or more** configured Redis index bindings — with discovery (`list-indexes`), explicit per-call routing (an optional `index` argument on `search-records`/`upsert-records`), and per-index write policy — while existing **single-index configs and callers behave exactly as before**. ## This is a roll-up — every change was already reviewed and merged This PR introduces **no new code**. It is the aggregate of a stacked series of per-ticket PRs, each reviewed and squash-merged into the `feature/raae-1603-mcp-multi-index` collector branch: - [#629](redis/redis-vl-python#629) — RAAE-1604: config + runtime refactor (immutable `BindingRuntime`, per-binding startup/teardown, `resolve_binding` routing, single global concurrency semaphore) - [#630](redis/redis-vl-python#630) — RAAE-1605: `list-indexes` discovery tool - [#631](redis/redis-vl-python#631) — RAAE-1606: `search-records` index routing - [#632](redis/redis-vl-python#632) — RAAE-1607: `upsert-records` index routing + per-index write policy - [#633](redis/redis-vl-python#633) — RAAE-1608: concept/how-to/README docs Each stacked PR was reviewed (human + Cursor Bugbot) and its threads resolved before merge; this branch is the sum of those merges plus a sync with `main`. ## Synced with `main` The branch is up to date with `main`, including the recently merged MCP security work — **Host/Origin header validation ([#643](redis/redis-vl-python#643 and the **nltk drop ([#645](redis/redis-vl-python#645 — pulled in via merge, so this PR's diff contains **only** the epic's own changes (main's commits are in the merge base). ## Security review A security review of the epic diff was run (auth scope enforcement, read-only/write policy, index routing, info leakage, injection, config/vectorizer resolution). **No findings** — tokenless HTTP is rejected upstream by the SDK auth middleware, scopes and read-only policy are enforced at both registration and per-call, routing is confined to configured bindings, and `redis_name` is never exposed by `list-indexes`. ## Verification - Full MCP unit + integration suite: **283 passed / 1 skipped** on `redis:8.2` (now runs without nltk). - `mypy` clean; `black`/`isort` clean. - Backward compatibility covered by dedicated tests (single-binding defaults, omitted-`index` behavior, unchanged response contract aside from the additive `index` echo). 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RAAE-1603]: https://redislabs.atlassian.net/browse/RAAE-1603?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes MCP server startup, routing, and write boundaries across bindings; single-index behavior is preserved but multi-index misconfiguration or routing errors could affect production assistants. > > **Overview** > **Multi-index MCP server** — One process can expose several logical index bindings from YAML `indexes`, each with its own schema inspection, search config, vectorizer, and runtime limits. Startup is all-or-nothing: any binding failure prevents the server from starting. > > **Routing and discovery** — New **`list-indexes`** tool enumerates logical ids, filterable fields, `upsert_available`, and optional limits (without exposing `redis_name`). **`search-records`** and **`upsert-records`** accept an optional **`index`** argument; responses echo the resolved id. With a single binding, omitting `index` still works; with multiple bindings, omitting or using an unknown id returns `invalid_request`. > > **Write policy** — Global `--read-only` / `REDISVL_MCP_READ_ONLY` plus per-binding **`read_only: true`** combine into **`effective_read_only`**. `upsert-records` is registered only if at least one binding is writable; writes to a read-only binding fail with **`forbidden`** before Redis is touched. > > **Implementation** — Config drops the “exactly one index” rule and adds **`description`** / **`read_only`** on bindings. Server state moves to per-binding **`BindingRuntime`** and **`resolve_binding()`**; removed single-index helpers like `get_index()`. Search tool descriptions omit per-schema hints when multiple indexes are configured and point clients to `list-indexes`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 553e55f4e93b306b643e3b8aca157e7a40a1497b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Motivation
The RedisVL MCP server currently binds to exactly one Redis index per process. That single-binding assumption is enforced by a config validator and baked throughout the codebase — single-resource server state, single-binding convenience accessors on
MCPConfig, and the search/upsert tools. Before the server can expose multiple logical indexes from a single endpoint (RAAE-1603), that assumption has to be removed and replaced with a real multi-binding model.This PR (RAAE-1604) does exactly that, and nothing more: it reshapes the configuration and runtime model so the server can start, inspect, validate, and serve one or many bindings, while keeping existing single-index configs and callers behaving identically. It is the foundation the rest of the epic (discovery via
list-indexes, index routing onsearch-records/upsert-records, docs) builds on, so it intentionally does not yet add any new request parameters or tools.Implementation
The core of the change is a new immutable
BindingRuntime(inredisvl/mcp/runtime.py) that bundles everything a tool call needs for one logical index: the binding config, the connectedAsyncSearchIndex, its effective (inspected + overridden) schema, an optional vectorizer, the resolved native-hybrid-search capability, and the effective read-only flag. The server now holds adict[str, BindingRuntime]keyed by logical id instead of a single set of_index/_vectorizerfields. Startup iterates every configured binding and inspects, validates, and initializes each one independently — each binding owns its own Redis client — with all-or-nothing teardown so a single bad binding fails startup cleanly without leaking connections.On the config side, the "exactly one configured index binding" validator is gone (we now simply require at least one binding with non-blank ids), and the schema-inspection, runtime-mapping, and search-validation methods move from
MCPConfigontoMCPIndexBindingConfigwhere they naturally belong per binding. The single-binding convenience accessors onMCPConfigare removed. Each binding gains optionaldescriptionandread_onlyfields, and a binding's effective write availability is computed as global--read-onlyOR the per-indexread_only. Tool resolution goes through a newserver.resolve_binding(index_id)helper that defaults to the sole binding when one is configured (preserving backward compatibility) and returns aninvalid_requesterror when an index is omitted with multiple bindings configured or when an unknown id is given. The search and upsert tools were re-threaded to operate on a resolvedBindingRuntimerather than reaching into single-binding server accessors.Additional notes:
BindingRuntime, replacing the previous lazy single-index cache.max_concurrencyacross bindings; the request timeout is sourced per-binding and passed explicitly intorun_guarded.get_index()/get_vectorizer()are retained as thin convenience wrappers overresolve_binding(None).description/read_onlydefaults,resolve_bindingrouting semantics, semaphore sizing, per-binding teardown, and three integration tests (multi-binding startup, global read-only override, and a single invalid binding failing startup), alongside the updated single-index tests that confirm backward compatibility.Verification
mypyclean across all source files;black/isortformatted.🤖 Generated with Claude Code
Note
Medium Risk
MCP startup/teardown and binding resolution affect how indexes are served; the required
run_guarded(..., timeout_seconds=)change breaks custom MCP extensions that call it directly.Overview
MCP moves from a single enforced index binding to a
dictofBindingRuntimeentries: startup inspects and initializes each configured index independently (own client, vectorizer, hybrid probe, effective read-only), withresolve_binding(index_id)defaulting when only one index is configured and rejecting ambiguous or unknown ids. Config drops the “exactly one binding” rule andMCPConfigconvenience accessors; per-bindingdescription,read_only, and schema/search helpers live onMCPIndexBindingConfig. Search/upsert tools read from the resolved runtime;run_guardednow requirestimeout_seconds=per binding (breaking for direct callers).Also in this release:
SearchIndex.drop_keysusesUNLINKinstead ofDEL; semantic routerdelete()removes the standalone route-config key;sql-redis>=0.7.1with docs forhybrid_vector_search/ FT.HYBRID; auto-release publishes to PyPI viapypa/gh-action-pypi-publishwith OIDC; version 0.22.0.Reviewed by Cursor Bugbot for commit bd2a28a. Bugbot is set up for automated code reviews on this repo. Configure here.
RedisVLMCPServer.run_guarded(operation_name, awaitable)now requires a keyword-onlytimeout_secondsargument (sourced from each binding'srequest_timeout_seconds). Any code that subclassesRedisVLMCPServeror callsrun_guardeddirectly (custom tools/plugins) must update its call sites to passtimeout_seconds=, otherwise it raisesTypeError: run_guarded() missing 1 required keyword-only argument: 'timeout_seconds'at call time. This repository has no CHANGELOG file, so this note serves as the migration callout (per review feedback on #629).