Repository navigation
feat(mcp): register code tools from Python with server.code_tool - #760
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: beb18035ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
limjoobin
left a comment
There was a problem hiding this comment.
LGTM, this PR connects the code-tool wrapper, which functions as an API for registering Python functions as MCP tools, to RedisVLMCPServer.
|
Looks good. Both Codex comments have been addressed, with regression coverage for the startup registration fix. Still LGTM, thanks! |
build_code_tool_spec checks a code tool where it is declared: it must be async, take a ToolContext first, annotate every other parameter, have a description, and carry a name the profile rules accept (now shared through validate_tool_name). register_code_tool hides the context from the advertised schema, checks the read or write scope and any per-tool scope before the function runs, bounds the whole call by the binding's request timeout, and maps exceptions to the MCP error contract.
A ToolContext declared keyword-only passed validation and then failed every call, because the wrapper passes it positionally; it is now refused where it is declared. A writing tool re-checks that its binding is still writable on each call, since tools register once while a restart re-reads the config. Name errors now point at the function, and the wrapper's tests cover the binding it resolves, a call over the wire and a timed-out call releasing its concurrency slot.
…close a queued call on timeout
server.custom_tool(...) registers an async function as an MCP tool on a server started from the caller's own process. At startup each tool is paired with its binding and refused when it cannot hold: a missing or unknown index, a name another custom tool uses, writes on a read-only binding or server, or any binding over a Redis index that a profile scopes with lock.inject, since nothing shows a code tool applies that scope. Code tools register after profiles and count towards the tool-surface fingerprint. ToolContext is now exported from redisvl.mcp, and the concepts and how-to guides document both.
…de tools server.custom_tool becomes server.code_tool, matching allow_code_tools and the docs, and no longer reading as a way to register a profile, which custom_tools already means. A restart that would pair the registered code tools with different bindings now fails instead of warning, since the old pairing would stay live. A writing tool on a read-only server says so rather than blaming the index. The docs describe the id-only writes and the SQLQuery refusal, replace an example that assumed the record carried its own id, and name code tools in the tool surface.
…and cast in the writing example
…tools; test the recorded pairing
694944f to
6f00412
Compare
…feat/mcp-code-tools/03-server-code-tool # Conflicts: # redisvl/mcp/tools/code.py
## Motivation
`server.code_tool(...)` from the previous PR only helps when the
caller's own code constructs the server. The common deployment is `rvl
mcp --config`, which imports no user code. This PR lets the config name
modules for the server to import. Because importing a module runs its
code with the server's privileges, nothing is imported unless the
operator opts in separately.
## Changes
`@code_tool(...)`, exported from `redisvl.mcp`, takes the same arguments
as `server.code_tool`. It checks the function and attaches its spec as
an attribute, without registering it anywhere:
```python
# myapp/support_tools.py
from redisvl.mcp import ToolContext, code_tool
@code_tool(index="tickets", namespace="support")
async def count_open_tickets(ctx: ToolContext, product: str) -> dict:
"""Count open support tickets for a product."""
...
```
```yaml
server:
redis_url: redis://localhost:6379
allow_code_tools: true
code_tool_modules: [myapp.support_tools]
```
At startup, the server imports each listed module before any binding
connects. Every function marked with `@code_tool` is validated and
registered alongside tools from `server.code_tool`, under the same
startup checks.
Startup refuses, naming the module or setting:
| Refused when | Why |
|---|---|
| `code_tool_modules` is set without `allow_code_tools: true` (default
`false`) | The import runs operator code; the opt-in keeps a
profiles-only config from running any Python. Checked at config load. |
| A module entry is blank, padded or listed twice | Almost certainly a
typo. |
| A module cannot be imported, or raises while importing | Reported by
module name, with the cause chained. |
| A module defines no `@code_tool` function | A misspelled module name
should not quietly produce a server with fewer tools. |
| A decorator is applied over `@code_tool` | `functools.wraps` copies
the marker while the spec still calls the inner function, so the outer
decorator would silently never run. |
The marker lives on the function, not in a module-level registry.
Importing a module therefore registers nothing anywhere, and two servers
in one process stay independent, as do tests that import a tool module.
The loader considers only functions and reads the marker from each
function's own `__dict__`, so a module attribute with a permissive
`__getattr__`, such as a lazy module proxy, is never consulted. A tool
one listed module re-exports from another registers once.
The docs gain "Code Tools Loaded From Modules" and a troubleshooting
entry. They state that `rvl` does not put the working directory on the
import path: modules must be installed or on `PYTHONPATH`, and under
`uvx` installed in the environment it builds.
## Notes
The loader does not edit `sys.path`. Adding the config's directory would
make import resolution depend on where the config file happens to live.
I verified the end-to-end flow manually with `rvl mcp --transport
streamable-http`, a scratch module on `PYTHONPATH` and the server
started from outside the project directory. A FastMCP client listed
`support-count-open-tickets` with only `product` in its schema, and a
call returned the expected count.
## Release Notes
`rvl mcp` can load code tools from Python modules. Mark async functions
with `@code_tool` from `redisvl.mcp`, list their modules under
`server.code_tool_modules`, and set `server.allow_code_tools: true`,
which defaults to `false` because importing a module runs its code with
the server's privileges. Modules must be importable by the server
process.
## Next Steps
- Update the custom-tools proposal to record where v1.1 departs from it:
the `code_tool` names, no `ctx.guarded` (the handle applies the limits
itself), id-only writes and no filter-wide bulk writes, no code tools on
tenant-scoped indexes, async tools only, and the whole-call timeout.
## Reviewer Brief
**Start here:** `redisvl/mcp/tools/code.py:269`, `load_code_tools`. It
imports the modules a config names, so it is where configuration becomes
running code.
**Where it could fail**
- `redisvl/mcp/config.py:266`: `allow_code_tools` is the only gate on
importing operator code; check no settings or environment path can set
`code_tool_modules` around it.
- `redisvl/mcp/tools/code.py:269`: the loader reads the `@code_tool`
marker from each function's own `__dict__` and refuses a decorator
wrapped over it; check a tool re-exported from a second module registers
once.
**Not verified:** importing modules under `uvx`, which the guide's
advice rests on reasoning about how `uvx` builds its environment;
FastMCP 2.x.
**Since the last brief:** `927124e` rebases the PR onto `main` as one
commit after #758, #759 and #760 were squash-merged; the code is
unchanged from the reviewed head plus its fixes.
<sub>Reviewed `6ff481b`, fixes in `cc9249f` · Claude Code
(claude-opus-5-5) · lenses: correctness, security, design, tests, docs,
Redis behaviour</sub>
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> When **`allow_code_tools`** is enabled, listed modules run with the
MCP server process privileges at import time; misconfiguration or
untrusted modules are the main exposure, though defaults stay off and
config validation fails closed on bad entries.
>
> **Overview**
> **`rvl mcp --config`** can now register custom code tools without
constructing **`RedisVLMCPServer`** in Python. Add **`@code_tool`**
(exported from **`redisvl.mcp`**, same options as
**`server.code_tool`**) on async functions; list modules under
**`server.code_tool_modules`** and set **`server.allow_code_tools:
true`** (defaults **`false`**).
>
> At startup—**before** any Redis binding connects—the server imports
each listed module via **`load_code_tools`**, collects functions marked
with **`__redisvl_mcp_tool__`**, and registers them through the same
validation path as tools added with **`server.code_tool`** (name
collisions, index binding, read-only/write rules, tenant
**`lock.inject`** routes). Config load rejects **`code_tool_modules`**
without the opt-in, duplicate or blank module names, failed imports,
modules with no **`@code_tool`** functions, and decorators wrapped
outside **`@code_tool`**.
>
> Docs add module-based registration, **`PYTHONPATH`** / **`uvx`**
import notes, and troubleshooting for module load failures.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
927124e. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Motivation
This PR makes code tools usable from a server started in the caller's own Python process. It builds on the tool wrapper from the previous PR:
The model sees
count-open-tickets, takingproduct, and never the context or the index.Changes
Registration
server.code_tool(...)checks the function at decoration and returns it unchanged. Registering after the first startup raisesRuntimeError, because tools register once per process. Code tools register after profiles.The method is named
code_toolrather thancustom_tool, which the proposal used.custom_tools:already means profiles in the config, and every message and setting calls these code tools.Startup checks
Each tool is paired with its binding once the config is loaded, before any binding connects. Startup fails, naming the tool, when:
indexis missing with several bindings, or names no bindinglock.injectredis_name, so a sibling binding over the same documents counts.The last check extends
_verify_no_unscoped_route_to_injected_indexesrather than duplicating its grouping, so the error lists a code tool alongside any other unscoped route.A restart that would pair the registered code tools with different bindings now fails, rather than taking the warning path other tool-surface changes take. The old pairing would stay live, and for operator code that is not something a log line should report.
Docs and exports
ToolContext,IndexHandleandWritableIndexHandleare exported fromredisvl.mcp; the guide shows thecasta type checker needs to see a writing tool's methods. The concepts page gains a Code Tools section covering what the context exposes and withholds, the rails applied to every call, and what fails at startup. The how-to guide gains "Code Tools Registered From Python", with a read tool and a guarded delete, plus a troubleshooting entry. The tool-surface and tenant-scoping sections now mention code tools, and a line saying custom tools never involve Python now says that of profiles specifically.Notes
Integration tests run against Redis 8:
A failed
ctx.index.loadreaches the client as a non-retryableinternal_error, even for a lost connection, becauseAsyncSearchIndex.loadwraps every error inRedisVLError. The built-inupsert-recordsbehaves the same way, so the docs state it rather than this PR changing the error mapping; that change belongs in its own PR, for both tools.Release Notes
Register an async Python function as an MCP tool with
server.code_tool(...)when you run the server from your own process. It reaches Redis only through aToolContextwhose handle stays inside the bound index, bounds every call, and offers writes only to tools declared withwrites=True.Next Steps
The final PR in the stack lets
rvl mcpload code tools from modules named in the config, behind an explicit opt-in.Reviewer Brief
Start here:
redisvl/mcp/server.py:457,_resolve_code_tools. It pairs each code tool with a binding once the config is loaded, and every startup refusal for code tools happens there or in the injection check it feeds.Where it could fail
redisvl/mcp/server.py:503: a code tool on any binding over aredis_namethat a profile scopes withlock.injectmust be refused, sibling bindings included; aliases and overlapping, empty or separator-only prefixes are outside this check by design.redisvl/mcp/server.py:645: a restart compares the new pairing with the recorded one; an unchanged config must compare equal, or every restart of a server with code tools fails.redisvl/mcp/server.py:260: registration must be refused once startup has begun, or a tool added while bindings connect is silently left out.Not verified: FastMCP 2.x; an in-process restart against a live Redis, which only the unit tests exercise.
Beyond CI: the guide's writing-tool example passes mypy as published; a mutant that stops recording the pairing fails the restart test; the Sphinx build at the #761 head adds no warnings to these pages.
Reviewed
09f1886, fixes in87e692f· Claude Code (claude-opus-5-5) · lenses: correctness, security, design, tests, docs, Redis behaviourNote
Medium Risk
Changes MCP tool surface and tenant-isolation startup rules; misconfiguration is fail-closed, but code tools still run operator Python with process privileges behind a bounded handle.
Overview
Adds code tools: async Python functions you register with
@server.code_tool(...)before starting a server from your own process, for logic YAML profiles cannot express.Registration and lifecycle: The decorator validates the function at decoration time and registers tools once at first startup (after profiles). Registering during or after startup is rejected. An in-process restart that changes which code tools exist or which index they bind to now fails with
RuntimeErrorinstead of silently keeping the old pairing.Startup policy: Each tool is resolved to a binding (required when multiple indexes exist), checked for name clashes with profiles, and refused if
writes=Trueon a read-only server or binding. Tenant-scoped indexes withlock.injectalso refuse any code tool on that Redis index, extending the existing “no unscoped route” check because the server cannot prove operator code applies the tenant filter.Public API:
ToolContext,IndexHandle, andWritableIndexHandleare exported fromredisvl.mcp. Docs add a Code Tools section (concepts + how-to) and troubleshooting, and tenant-scoping text now mentions code tools as alternate routes to the same data.Reviewed by Cursor Bugbot for commit d6e71eb. Bugbot is set up for automated code reviews on this repo. Configure here.