Skip to content

feat(mcp): add index routing + per-index write policy to upsert-records (RAAE-1607)#632

Merged
vishal-bala merged 7 commits into
feature/raae-1603-mcp-multi-indexfrom
feature/raae-1607-upsert-routing
Jul 2, 2026
Merged

feat(mcp): add index routing + per-index write policy to upsert-records (RAAE-1607)#632
vishal-bala merged 7 commits into
feature/raae-1603-mcp-multi-indexfrom
feature/raae-1607-upsert-routing

Conversation

@vishal-bala

@vishal-bala vishal-bala commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Motivation

This completes the multi-index tool surface for the RedisVL MCP server. After RAAE-1606 taught search-records to route by logical index, upsert-records (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#630#631 → this PR.

🤖 Generated with Claude Code


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.

Reviewed by Cursor Bugbot for commit 98aded7. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci

jit-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@nkanu17 nkanu17 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@nkanu17 nkanu17 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

vishal-bala and others added 3 commits June 30, 2026 22:16
Add an always-registered, read-only `list-indexes` MCP tool so clients can
enumerate the logical indexes a multi-index server exposes and choose the
right one before calling search-records or upsert-records.

For each configured binding the tool returns the logical id, an optional
description, whether upsert is available (reflecting both the global
--read-only flag and the per-index read_only policy), the shared filterable
fields, and any explicitly configured runtime limits. Fields are derived from
the binding's already-inspected effective schema rather than user-declared
metadata; the vector field and the configured default embed-source text field
are omitted because they are implementation inputs, not things a client
filters on. The Redis index name (redis_name) is never exposed. Limits are
surfaced only when explicitly set in config (detected via the runtime model's
model_fields_set), so the output reflects deliberate overrides rather than
defaults.

- New redisvl/mcp/tools/list_indexes.py with list_indexes() + register_list_indexes_tool().
- Registered unconditionally in the server's tool registration, alongside search/upsert.
- Output is deterministic and ordered by configured binding.
- TDD: unit coverage for field omission, description/limits inclusion rules,
  redis_name secrecy, read-only reflection, and registration; integration test
  verifying fields are derived from the inspected schema across a vector and a
  fulltext binding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the opaque `rt: Any` parameters in list_indexes.py with the concrete
`BindingRuntime` type and the clearer name `binding_runtime`, and type the
`server` parameters as `RedisVLMCPServer` (via a TYPE_CHECKING import to avoid
the server<->tools import cycle). No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ead (RAAE-1605)

Address review on list_indexes.py:

- Remove the `tool_list_indexes_description` override: that setting does not
  exist on MCPSettings (only tool_search/upsert_description do), so the getattr
  branch was always None and never fired. Pass the default description constant
  directly.
- Read the read scope as `auth_config.read_scope` (a typed field on
  MCPAuthConfig) instead of a silent `getattr(..., "read_scope", None)`. The
  old form would fail open — silently yielding None and skipping auth
  enforcement — if the field were ever renamed; direct access fails loud.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vishal-bala vishal-bala force-pushed the feature/raae-1606-search-routing branch from 80a6a78 to 3d41055 Compare June 30, 2026 20:20
@vishal-bala vishal-bala force-pushed the feature/raae-1607-upsert-routing branch from e92fb9a to 2f878a9 Compare June 30, 2026 20:30
vishal-bala and others added 2 commits July 1, 2026 11:19
…605)

list_indexes registered the tool instance-level, so it can still be called
before startup or after shutdown when _bindings is empty. Returning
{"indexes": []} there is misleading — a client reads it as "no indexes
configured" rather than "server not ready". Guard with the same
"MCP server has not been started" RuntimeError that resolve_binding raises.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an optional `index` argument to the search-records tool so a single
multi-binding MCP server can target a specific logical index. The argument
is optional when exactly one binding is configured (preserving single-index
behavior) and resolves through the same resolve_binding routing used
elsewhere, so an omitted index on a multi-binding server and unknown ids both
surface as invalid_request. The resolved logical id is echoed back as the
`index` field in the response.

- Expose `index` on the FastMCP wrapper param list.
- Append a routing note to the tool description when the schema is ambiguous
  (multiple bindings) directing clients to call list-indexes first.
- Add unit + integration coverage for routing, omitted-index rejection,
  unknown ids, and single-binding backward compatibility.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vishal-bala vishal-bala force-pushed the feature/raae-1606-search-routing branch from 3d41055 to c2f30d5 Compare July 1, 2026 09:21
vishal-bala and others added 2 commits July 1, 2026 11:22
…ords (RAAE-1607)

Add an optional `index` argument to the upsert-records tool so a multi-binding
MCP server can target a specific logical index for writes. As with search, the
argument is optional on single-binding servers and required when multiple
bindings exist; resolution flows through the shared resolve_binding routing so
an omitted index on a multi-binding server and unknown ids both surface as
invalid_request. The resolved logical id is echoed back as the `index` field in
the response, and the selected binding's embedding, runtime limits, and schema
validation are used throughout.

Write availability is now enforced at two levels. The upsert tool is registered
only when at least one binding is writable, so an all-read-only server (whether
from global read-only mode or every binding's own read_only policy) does not
advertise the tool at all. When the tool is registered, a per-call check
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.

- Expose `index` on the FastMCP wrapper param list.
- Refine the registration gate from "global read-only off" to "any binding
  writable" using effective_read_only, which folds in both global and
  per-index read-only.
- Add unit + integration coverage for routing, omitted-index rejection,
  unknown ids, read-only rejection, the registration gate, and single-binding
  backward compatibility.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align the read-only write rejection with the FORBIDDEN error code used on the
1604 branch, instead of INVALID_REQUEST. A read-only binding is a permission
policy denial (the request is well-formed but not allowed), so FORBIDDEN is the
correct category; INVALID_REQUEST remains for malformed/unroutable requests
(unknown index, omitted index, bad shapes). Keeps the two stacked branches
consistent so they reconcile cleanly when the stack is collected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vishal-bala vishal-bala force-pushed the feature/raae-1607-upsert-routing branch from 2f878a9 to 98aded7 Compare July 1, 2026 09:22
@vishal-bala vishal-bala marked this pull request as ready for review July 1, 2026 09:43
Base automatically changed from feature/raae-1606-search-routing to feature/raae-1603-mcp-multi-index July 2, 2026 11:38
@vishal-bala

Copy link
Copy Markdown
Collaborator Author

Failing tests are from the Redis 8.8 race condition issue that's been flagged before

@vishal-bala vishal-bala merged commit df52819 into feature/raae-1603-mcp-multi-index Jul 2, 2026
54 of 57 checks passed
@vishal-bala vishal-bala deleted the feature/raae-1607-upsert-routing branch July 2, 2026 11:41
vishal-bala added a commit that referenced this pull request Jul 2, 2026
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants