Skip to content

feat(mcp): validate and wrap async functions as code tools - #759

Draft
vishal-bala wants to merge 6 commits into
feat/mcp-code-tools/01-tool-contextfrom
feat/mcp-code-tools/02-code-tool-wrapper
Draft

vishal-bala wants to merge 6 commits into
feat/mcp-code-tools/01-tool-contextfrom
feat/mcp-code-tools/02-code-tool-wrapper

Conversation

@vishal-bala

@vishal-bala vishal-bala commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

A code tool is an async function whose first parameter is a ToolContext and 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 the ToolContext from 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 frozen CodeToolSpec. It refuses:

  • a sync function, since ctx.index is async and a sync tool could not use it;
  • a first parameter that is not a positional ToolContext. The wrapper passes the context positionally, so a keyword-only one would pass a shape check and then fail on every call;
  • any other parameter that is unannotated, *args, **kwargs or positional-only. The annotations become the tool's input schema;
  • a parameter type that no input schema can describe, such as a plain class. Otherwise it would surface only at startup, after every binding had connected;
  • a missing description. The docstring is used unless 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 by namespace= when given. It is checked by validate_tool_name, now factored out of profile validation, so profiles and code tools share one naming rule. Annotations are resolved with typing.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 how register_profile_tool narrows 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:

  1. checks the server's read scope, or the write scope for a tool declared with writes=True, then the tool's own scope= on top. A tool can therefore be stricter than the built-ins, never looser;
  2. refuses a writing tool whose binding is now read-only;
  3. builds the ToolContext, giving a writing tool the writable handle;
  4. bounds the whole call by the binding's request_timeout_seconds, so the tool's own code cannot hold a request open indefinitely. A timeout returns a retryable backend_unavailable that names the tool;
  5. maps any other exception through map_exception, so a ValueError reaches the client as invalid_request and no traceback leaks. A raised RedisVLMCPError passes 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_guarded now 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-tool scope=, 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 d6e0154 by Claude Code (claude-opus-5-5) with pre-pr-review; fixes in 26130ee.
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.

  • 7 findings raised here, 4 kept after challenge.
  • Fixed: a parameter type with no JSON schema passed declaration and failed only at startup, after the bindings connected (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.
  • Rejected: shutdown can land between two handle calls of a multi-step tool (needs an embedder to stop the server mid-call, and multi-step writes are not atomic under the whole-call timeout either); a 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); a TimeoutError raised inside the tool is reported as the whole-call timeout (same error code, only the message differs).
  • Open for the reviewer: none.

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 the run_guarded fix each fail the new tests.
Not checked: FastMCP 2.x, which the >=2.0.0 floor 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.

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.
@vishal-bala
vishal-bala added this pull request to stack #762 October 14, 2026 09:01
@vishal-bala vishal-bala added the auto:minor Increment the minor version when merged label Oct 15, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:minor Increment the minor version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant