Repository navigation
feat(mcp): validate and wrap async functions as code tools - #759
Draft
vishal-bala wants to merge 6 commits into
Draft
vishal-bala wants to merge 6 commits into
vishal-bala wants to merge 6 commits into
Conversation
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.
…-tools/02-code-tool-wrapper
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.
vishal-bala
added this pull request to stack #762
October 14, 2026 09:01
…-tools/02-code-tool-wrapper
…-tools/02-code-tool-wrapper
…close a queued call on timeout
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A code tool is an async function whose first parameter is a
ToolContextand whose other parameters are the arguments the model supplies. This PR turns such a function into a registered MCP tool, with the same rails a built-in gets, applied around the function rather than left to its author. It builds on theToolContextfrom the previous PR. Users still cannot reach it, because the server registers nothing yet.Changes
Checked where the tool is declared
build_code_tool_spec(fn, ...)validates a function and returns a frozenCodeToolSpec. It refuses:ctx.indexis async and a sync tool could not use it;ToolContext. The wrapper passes the context positionally, so a keyword-only one would pass a shape check and then fail on every call;*args,**kwargsor positional-only. The annotations become the tool's input schema;description=is given, because the description is what the model selects tools by.The name is the function's name with hyphens for underscores, or
name=, prefixed bynamespace=when given. It is checked byvalidate_tool_name, now factored out of profile validation, so profiles and code tools share one naming rule. Annotations are resolved withtyping.get_type_hints, so a module using postponed annotations is checked against the real types.Applied on every call
register_code_tool(server, spec, binding_id)registers a wrapper whose advertised signature is the function's minus the context. This follows howregister_profile_toolnarrows its signature, so FastMCP never offers the context to the model, and it rejects a call that tries to pass one. On each call the wrapper:writes=True, then the tool's ownscope=on top. A tool can therefore be stricter than the built-ins, never looser;ToolContext, giving a writing tool the writable handle;request_timeout_seconds, so the tool's own code cannot hold a request open indefinitely. A timeout returns a retryablebackend_unavailablethat names the tool;map_exception, so aValueErrorreaches the client asinvalid_requestand no traceback leaks. A raisedRedisVLMCPErrorpasses through unchanged.Startup will also refuse a writing tool on a read-only binding (next PR). The per-call check in step 2 is defence in depth, as in
upsert_records, so a write never depends on that one check.Notes
The whole-call timeout and the per-call handle timeout use the same value, so the outer bound usually fires first. That is deliberate: the per-call bound protects the semaphore, and the outer bound protects the request.
A test runs a timed-out call against the real
run_guarded. It checks that the call releases its concurrency slot and the active-request count, since a leak there would remove one slot per timeout until the server stopped answering.The whole-call timeout can also cancel a handle call while it is still queued for a concurrency slot.
run_guardednow acquires the slot explicitly and closes the Redis call it never started, which previously logged "coroutine was never awaited" once per such timeout.Release Notes
An async function becomes a code tool only if it takes a positional
ToolContext, annotates its other parameters and has a description; every call is then checked against the read or write scope plus any per-toolscope=, bounded by the binding's request timeout, and mapped to the MCP error contract.Next Steps
The next PR in the stack adds
server.code_tool(...)and the startup checks that pair each tool with its binding.Review record
Reviewed at
d6e0154by Claude Code (claude-opus-5-5) withpre-pr-review; fixes in26130ee.Lenses: correctness (always); security (scope gates and write gating in
redisvl/mcp/); design (public spec and registration API); tests (adds unit tests); docs (docstrings and comments); Redis behaviour (stack-wide pass, nothing in this PR). The lenses ran over the whole stack, and findings were attributed to the PR that introduced the line.tools/code.py:103); a call cancelled while queued for the concurrency slot left its Redis coroutine unclosed (server.py:270); the per-call read-only comment described a restart path that startup already refuses (tools/code.py:195); the scope-refusal test omitted the tool scope, so deleting the server-wide scope check passed every test (test_code_tools_unit.py:337). Each regression test fails without its fix.scope=is not enforced without authentication (consistent with the server-wide scopes, and refusing it would break scoped tools on stdio; the docs now say so, in feat(mcp): register code tools from Python with server.code_tool #760); aTimeoutErrorraised inside the tool is reported as the whole-call timeout (same error code, only the message differs).Verified:
pytest tests/unit/test_mcp(548 passed);pytest -n 1 tests/integration/test_mcp(80 passed, 2 skipped, Redis 8);mypy redisvl/mcp(no issues); mutants deleting the server-wide scope check and reverting therun_guardedfix each fail the new tests.Not checked: FastMCP 2.x, which the
>=2.0.0floor allows. In particular,get_access_token()now runs on every code-tool call, including over stdio; it was tested on FastMCP 3.1.0 and 3.3.1 only.