Skip to content

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

Open
vishal-bala wants to merge 2 commits into
feature/raae-1606-search-routingfrom
feature/raae-1607-upsert-routing
Open

feat(mcp): add index routing + per-index write policy to upsert-records (RAAE-1607)#632
vishal-bala wants to merge 2 commits into
feature/raae-1606-search-routingfrom
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 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 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
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