[Draft] Convert BulkToolCaller to middleware #2267
Conversation
- Create BulkToolCallerMiddleware extending ToolInjectionMiddleware - Add bulk_tool_caller_types.py to avoid circular imports - Deprecate original BulkToolCaller class with migration guidance - Maintain backward compatibility by re-exporting types - Add comprehensive test suite for middleware implementation - Use lazy import in middleware __init__.py to prevent circular dependencies Both approaches are now available: - New: FastMCP(middleware=[BulkToolCallerMiddleware()]) - Old (deprecated): BulkToolCaller().register_tools(mcp) Co-authored-by: William Easton <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR converts the BulkToolCaller from an MCPMixin-based implementation to a ToolInjectionMiddleware, simplifying usage and aligning with the architecture established in prior middleware implementations.
Key Changes:
- Introduced
BulkToolCallerMiddlewarethat extendsToolInjectionMiddleware - Deprecated the original
BulkToolCallerclass with migration guidance - Created a new types module to avoid circular imports
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/server/middleware/test_bulk_tool_caller.py | Comprehensive test suite for the new middleware implementation and deprecation behavior |
| src/fastmcp/server/middleware/bulk_tool_caller_types.py | New types module containing CallToolRequest and CallToolRequestResult to avoid circular dependencies |
| src/fastmcp/server/middleware/bulk_tool_caller.py | New middleware implementation with call_tools_bulk and call_tool_bulk functions |
| src/fastmcp/server/middleware/init.py | Lazy loading support for BulkToolCallerMiddleware |
| src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py | Deprecation warnings added and types re-exported for backward compatibility |
| """A simple tool that returns nothing.""" | ||
|
|
There was a problem hiding this comment.
The no_return_tool function is missing a docstring body. While it has a summary line, functions that intentionally return None should document this behavior explicitly.
| """A simple tool that returns nothing.""" | |
| """A simple tool that returns nothing. | |
| Returns: | |
| None: This tool does not return any value. | |
| """ |
| tool=tool, | ||
| arguments=arguments, | ||
| isError=result.isError, | ||
| content=result.content, |
There was a problem hiding this comment.
The from_call_tool_result method doesn't copy the _meta or structuredContent fields from the source CallToolResult. If these fields contain values, they will be lost in the conversion. Consider including these fields: _meta=result._meta, structuredContent=result.structuredContent
| content=result.content, | |
| content=result.content, | |
| _meta=getattr(result, "_meta", None), | |
| structuredContent=getattr(result, "structuredContent", None), |
| call_tool_result = CallToolResult( | ||
| content=tool_result.content, | ||
| ) |
There was a problem hiding this comment.
The intermediate CallToolResult creation is unnecessary. You can pass tool_result.content directly to CallToolRequestResult.from_call_tool_result or create the CallToolRequestResult directly from tool_result, eliminating this temporary object.
| call_tool_result = CallToolResult( | ||
| content=tool_result.content, | ||
| ) |
There was a problem hiding this comment.
The intermediate CallToolResult creation is unnecessary. You can pass tool_result.content directly to CallToolRequestResult.from_call_tool_result or create the CallToolRequestResult directly from tool_result, eliminating this temporary object.
marvin-context-protocol
bot
added
enhancement
|
@coderabbitai please review |
|
@strawgate: I'll review the changes in this PR right away. ✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughConverts the bulk tool caller into middleware: introduces BulkToolCallerMiddleware with injectable tools Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant BulkMiddleware as BulkToolCallerMiddleware
participant ToolManager
rect rgb(230, 245, 255)
Note over Client,ToolManager: Middleware flow (call_tool_bulk / call_tools_bulk)
Client->>Server: invoke bulk tool (call_tool_bulk / call_tools_bulk)
Server->>BulkMiddleware: dispatch to injected tool
loop for each call
BulkMiddleware->>ToolManager: call tool(name, arguments)
alt success
ToolManager-->>BulkMiddleware: ToolResult
BulkMiddleware->>BulkMiddleware: convert ToolResult -> CallToolRequestResult
else exception
ToolManager-->>BulkMiddleware: Exception
BulkMiddleware->>BulkMiddleware: create error CallToolRequestResult
alt continue_on_error = false
BulkMiddleware->>BulkMiddleware: break loop
end
end
end
BulkMiddleware-->>Server: list[CallToolRequestResult]
Server-->>Client: aggregated results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
tests/server/middleware/test_bulk_tool_caller.py (1)
31-33: Document the None return explicitly in the tool docstringNote that the tool intentionally returns None to aid schema/docs clarity.
async def no_return_tool(arg1: str) -> None: - """A simple tool that returns nothing.""" + """A simple tool that returns nothing. + + Returns: + None: This tool does not return any value. + """src/fastmcp/server/middleware/bulk_tool_caller_types.py (1)
29-39: Preserve structuredContent and _meta when converting resultsfrom_call_tool_result drops structuredContent and _meta, losing upstream data. Copy them through.
@classmethod def from_call_tool_result( cls, result: CallToolResult, tool: str, arguments: dict[str, Any] ) -> "CallToolRequestResult": """Create a CallToolRequestResult from a CallToolResult.""" - return cls( + return cls( tool=tool, arguments=arguments, isError=result.isError, - content=result.content, + content=result.content, + _meta=getattr(result, "_meta", None), + # Support either camelCase or snake_case attribute names + structuredContent=getattr( + result, "structuredContent", getattr(result, "structured_content", None) + ), )src/fastmcp/server/middleware/bulk_tool_caller.py (2)
51-60: Avoid intermediate CallToolResult and preserve all fieldsThe temporary CallToolResult drops structuredContent/_meta. Build CallToolRequestResult directly from tool_result to keep everything intact.
- # Convert ToolResult to CallToolResult - # Don't set isError - it defaults to None for successful calls - call_tool_result = CallToolResult( - content=tool_result.content, - ) - - # Convert to CallToolRequestResult - results.append( - CallToolRequestResult.from_call_tool_result( - call_tool_result, tool_call.tool, tool_call.arguments - ) - ) + # Convert to CallToolRequestResult, preserving all available fields + results.append( + CallToolRequestResult( + tool=tool_call.tool, + arguments=tool_call.arguments, + isError=getattr(tool_result, "isError", None), + content=tool_result.content, + structuredContent=getattr( + tool_result, + "structuredContent", + getattr(tool_result, "structured_content", None), + ), + _meta=getattr(tool_result, "_meta", None), + ) + )
116-125: Apply the same preservation fix in call_tool_bulkMirror the direct construction to keep structuredContent/_meta here as well.
- # Convert ToolResult to CallToolResult - # Don't set isError - it defaults to None for successful calls - call_tool_result = CallToolResult( - content=tool_result.content, - ) - - # Convert to CallToolRequestResult - results.append( - CallToolRequestResult.from_call_tool_result( - call_tool_result, tool, args - ) - ) + results.append( + CallToolRequestResult( + tool=tool, + arguments=args, + isError=getattr(tool_result, "isError", None), + content=tool_result.content, + structuredContent=getattr( + tool_result, + "structuredContent", + getattr(tool_result, "structured_content", None), + ), + _meta=getattr(tool_result, "_meta", None), + ) + )
🧹 Nitpick comments (4)
src/fastmcp/server/middleware/__init__.py (1)
8-13: Improve static typing and discoverability for the lazy exportAdd a TYPE_CHECKING import and a dir implementation so type checkers and IDEs see BulkToolCallerMiddleware, and dir() lists it.
+from typing import TYPE_CHECKING + +if TYPE_CHECKING: # Help static analyzers/IDEs + from .bulk_tool_caller import BulkToolCallerMiddleware + def __getattr__(name: str): if name == "BulkToolCallerMiddleware": from .bulk_tool_caller import BulkToolCallerMiddleware return BulkToolCallerMiddleware raise AttributeError(f"module {__name__!r} has no attribute {name!r}") +def __dir__() -> list[str]: + # Ensure "BulkToolCallerMiddleware" shows up in dir(fastmcp.server.middleware) + return sorted(list(globals().keys()) + ["BulkToolCallerMiddleware"]) __all__ = [ "BulkToolCallerMiddleware", "CallNext", "Middleware", "MiddlewareContext", ]Also applies to: 16-21
tests/server/middleware/test_bulk_tool_caller.py (1)
52-58: Avoid brittle count assertion in tool listingThe exact length check can flake if the server injects extra tools. Keep name membership checks and drop the length assertion.
- # Should have: echo_tool, error_tool, no_return_tool, add, call_tools_bulk, call_tool_bulk - assert len(tools) == 6 + # Expect these tools to be present regardless of other server-injected tools assert "call_tools_bulk" in tool_names assert "call_tool_bulk" in tool_names assert "echo_tool" in tool_names assert "error_tool" in tool_names assert "no_return_tool" in tool_names assert "add" in tool_namessrc/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (1)
118-126: Preserve structuredContent/_meta by using the helper constructorLeverage CallToolRequestResult.from_call_tool_result to keep all fields (including structuredContent and _meta) instead of re‑constructing manually.
- async with Client(self.connection) as client: - result = await client.call_tool_mcp(name=tool, arguments=arguments) - - return CallToolRequestResult( - tool=tool, - arguments=arguments, - isError=result.isError, - content=result.content, - ) + async with Client(self.connection) as client: + call_result = await client.call_tool_mcp(name=tool, arguments=arguments) + return CallToolRequestResult.from_call_tool_result( + call_result, tool=tool, arguments=arguments + )src/fastmcp/server/middleware/bulk_tool_caller.py (1)
16-26: Optional: add bounded concurrency to speed up bulk callsIf tools are I/O‑bound and independent, consider an optional max_concurrency to run calls with asyncio.Semaphore/gather.
-async def call_tools_bulk( +async def call_tools_bulk( context: Context, tool_calls: Annotated[ list[CallToolRequest], "List of tool calls to execute. Each call can be for a different tool with different arguments.", ], continue_on_error: Annotated[ bool, "If True, continue executing remaining tools even if one fails. If False, stop on first error.", - ] = True, + ] = True, + max_concurrency: int | None = None, ) -> list[CallToolRequestResult]: @@ - results = [] - - for tool_call in tool_calls: + results: list[CallToolRequestResult] = [] + sem = None if not max_concurrency else __import__("asyncio").Semaphore(max_concurrency) + + async def _one(tc: CallToolRequest) -> CallToolRequestResult: + if sem: + async with sem: + return await _call_single(context, tc.tool, tc.arguments) + return await _call_single(context, tc.tool, tc.arguments) + + for tool_call in tool_calls: try: - # Call the tool directly through the tool manager - tool_result = await context.fastmcp._tool_manager.call_tool( - key=tool_call.tool, arguments=tool_call.arguments - ) + tool_result = await context.fastmcp._tool_manager.call_tool( + key=tool_call.tool, arguments=tool_call.arguments + )Note: Same pattern can be applied to call_tool_bulk.
Also applies to: 79-90
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py(3 hunks)src/fastmcp/server/middleware/__init__.py(1 hunks)src/fastmcp/server/middleware/bulk_tool_caller.py(1 hunks)src/fastmcp/server/middleware/bulk_tool_caller_types.py(1 hunks)tests/server/middleware/test_bulk_tool_caller.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (2)
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (1)
23-37: Confirm the deprecation version tagThe docstring marks deprecation as 2.1.0. Please confirm this matches the planned release that ships the middleware, or adjust before merge.
tests/server/middleware/test_bulk_tool_caller.py (1)
1-10: Async test configuration is correctThe pytest configuration in
pyproject.tomlexplicitly setsasyncio_mode = "auto"and includespytest-asyncio>=1.2.0. This configuration automatically detects and runs async tests without requiring@pytest.mark.asynciodecorators. The async tests intests/server/middleware/test_bulk_tool_caller.pywill execute correctly as configured.
|
/marvin can you please address the PR feedback |
|
Claude finished @strawgate's task —— View job Addressing PR Feedback ✅Successfully addressed all code review feedback from both reviewers. Changes Made
SummaryAll reviewer feedback has been addressed:
All pre-commit checks pass and all 13 tests pass successfully. --- | |
- Fix no_return_tool docstring to document None return - Preserve _meta and structuredContent in CallToolRequestResult - Remove intermediate CallToolResult creation - Use from_call_tool_result helper in deprecated code - Add TYPE_CHECKING import and __dir__ for better IDE support - Update test snapshots to reflect structuredContent preservation Co-authored-by: William Easton <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/fastmcp/server/middleware/bulk_tool_caller.py (1)
109-111: Same filtering bypass issue as in call_tools_bulk.This call has the same security implications as noted in call_tools_bulk (lines 45-47).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py(4 hunks)src/fastmcp/server/middleware/__init__.py(1 hunks)src/fastmcp/server/middleware/bulk_tool_caller.py(1 hunks)src/fastmcp/server/middleware/bulk_tool_caller_types.py(1 hunks)tests/server/middleware/test_bulk_tool_caller.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fastmcp/server/middleware/bulk_tool_caller_types.py
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use Python ≥ 3.10 and provide full type annotations for library code
Files:
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.pysrc/fastmcp/server/middleware/bulk_tool_caller.pysrc/fastmcp/server/middleware/__init__.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Never use bare except; always catch specific exception types
Files:
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.pysrc/fastmcp/server/middleware/bulk_tool_caller.pytests/server/middleware/test_bulk_tool_caller.pysrc/fastmcp/server/middleware/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Tests must be atomic, self-contained, and cover a single functionality
Use pytest parameterization for multiple examples of the same functionality
Use separate tests for distinct pieces of functionality
Always put imports at the top of test files; do not import inside test bodies
Do not add @pytest.mark.asyncio; asyncio_mode = "auto" is set globally
Prefer in-memory transport for tests; use HTTP transport only when explicitly testing networking
For slow/long-running tests, mark them as integration or optimize (default timeout is 5s)
In tests, use # type: ignore[attr-defined] for MCP results instead of type assertions
Files:
tests/server/middleware/test_bulk_tool_caller.py
🧬 Code graph analysis (4)
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (4)
src/fastmcp/server/middleware/bulk_tool_caller_types.py (3)
CallToolRequest(9-15)CallToolRequestResult(18-41)from_call_tool_result(30-41)src/fastmcp/contrib/mcp_mixin/mcp_mixin.py (1)
MCPMixin(118-276)src/fastmcp/server/server.py (1)
_call_tool_middleware(1074-1092)tests/contrib/test_bulk_tool_caller.py (1)
bulk_caller_live(81-85)
src/fastmcp/server/middleware/bulk_tool_caller.py (6)
src/fastmcp/server/context.py (1)
Context(103-692)src/fastmcp/server/middleware/bulk_tool_caller_types.py (2)
CallToolRequest(9-15)CallToolRequestResult(18-41)src/fastmcp/server/middleware/tool_injection.py (1)
ToolInjectionMiddleware(21-51)src/fastmcp/tools/tool.py (1)
Tool(105-241)src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (2)
call_tools_bulk(63-81)call_tool_bulk(84-109)src/fastmcp/server/server.py (2)
_call_tool_mcp(1046-1072)_call_tool_middleware(1074-1092)
tests/server/middleware/test_bulk_tool_caller.py (3)
src/fastmcp/server/server.py (2)
FastMCP(148-2693)name(359-360)src/fastmcp/server/middleware/bulk_tool_caller.py (1)
BulkToolCallerMiddleware(142-192)src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (2)
BulkToolCaller(20-123)register_tools(39-60)
src/fastmcp/server/middleware/__init__.py (1)
src/fastmcp/server/middleware/bulk_tool_caller.py (1)
BulkToolCallerMiddleware(142-192)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (6)
src/fastmcp/server/middleware/__init__.py (1)
1-24: LGTM! Well-implemented lazy import pattern.The lazy import implementation using
__getattr__and__dir__follows PEP 562 conventions correctly, enabling optional middleware without forcing import-time dependencies.tests/server/middleware/test_bulk_tool_caller.py (1)
1-305: LGTM! Comprehensive test coverage.The test suite follows all coding guidelines:
- Proper imports at the top
- Atomic, self-contained tests
- Correct use of
# type: ignore[attr-defined]for MCP results- No unnecessary
@pytest.mark.asynciodecorators- Good coverage of success, error, filtering, and deprecation scenarios
src/fastmcp/contrib/bulk_tool_caller/bulk_tool_caller.py (1)
1-123: LGTM! Well-executed deprecation strategy.The deprecation is properly implemented with:
- Clear version notice in docstring (
.. deprecated:: 2.1.0)- Runtime warning with migration guidance
- Backward compatibility via type re-exports
- Correct
stacklevel=2pointing to the callersrc/fastmcp/server/middleware/bulk_tool_caller.py (3)
60-70: Appropriate exception handling for bulk operations.Catching
Exception(not bareexcept:) is acceptable here because:
- Bulk operations need to handle errors from arbitrary tools
- Errors are properly reported to the caller with
isError=True- The error message preserves the original exception information
142-192: Well-structured middleware with clear documentation.The class structure and documentation are excellent:
- Clear inheritance from ToolInjectionMiddleware
- Comprehensive docstring with usage examples
- Proper tool registration via
Tool.from_function()
45-47: Bypass of tool filtering is confirmed but appears intentional—test coverage incomplete.The review comment is correct:
bulk_tool_caller.pyline 45-47 directly callscontext.fastmcp._tool_manager.call_tool(), which bypasses the_should_enable_component()filtering that the proper middleware chain (_call_tool_middleware()) applies.The test file includes
test_bulk_tools_bypass_filtering(), which explicitly acknowledges this behavior. However, the test is incomplete: it verifies that filtered tools don't appear in the tool list while bulk tools remain available, but it does not verify whether:
- Calling a filtered-out tool through bulk tools actually succeeds (security gap)
- This bypass is intentional or a bug
- What the expected behavior should be
Recommendation: Update the test to clarify intent by adding an assertion that verifies whether calling a filtered tool via bulk tools succeeds or fails. If the bypass is intentional, document it prominently with a comment explaining why filtering is not applied to bulk operations. If filtering should be respected, update the implementation to use
_call_tool_middleware()instead.
| async def test_bulk_tools_bypass_filtering(self): | ||
| """Test that bulk caller tools bypass tag filtering.""" | ||
| mcp = FastMCP( | ||
| "FilteredServer", | ||
| middleware=[BulkToolCallerMiddleware()], | ||
| exclude_tags={"math"}, | ||
| ) | ||
|
|
||
| @mcp.tool(tags={"math"}) | ||
| def multiply(a: int, b: int) -> int: | ||
| """Multiply two numbers.""" | ||
| return a * b | ||
|
|
||
| async with Client(mcp) as client: | ||
| tools = await client.list_tools() | ||
|
|
||
| tool_names = [tool.name for tool in tools] | ||
| # The multiply tool should be filtered out, but bulk tools should still be available | ||
| assert "call_tools_bulk" in tool_names | ||
| assert "call_tool_bulk" in tool_names | ||
| assert "multiply" not in tool_names | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Verify that bulk tools respect filtering and disabled tool states.
The test confirms bulk tools themselves bypass filtering, but doesn't verify whether bulk tools can invoke filtered-out or disabled tools. This could be a security concern if tools are disabled for a reason (e.g., security, authorization).
Run the following script to check how bulk tools interact with filtering:
Consider adding a test case that attempts to call a filtered-out tool via bulk tools to document the expected behavior.
🏁 Script executed:
#!/bin/bash
# Description: Check if bulk tools can call disabled/filtered tools
# Search for _should_enable_component usage in bulk tool implementation
rg -nP --type=py -C5 '_should_enable_component|_tool_manager\.call_tool' src/fastmcp/server/middleware/bulk_tool_caller.py
# Check if filtering logic is applied in the tool manager
ast-grep --pattern 'def call_tool($$$) {
$$$
}'Length of output: 939
I need to investigate the tool manager's implementation to determine if filtering is applied when tools are called directly. Let me search for the relevant code.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the tool manager implementation and its call_tool method
fd --type=f -e py "tool_manager" src/ | head -20
# Look for the call_tool method definition
rg -nP --type=py 'def call_tool\s*\(' src/
Length of output: 325
🏁 Script executed:
#!/bin/bash
# Search for _should_enable_component or filtering checks in call_tool
rg -nP --type=py -A20 'def call_tool\s*\(' src/Length of output: 4908
I need to check the get_tool method and understand how filtering is implemented.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find get_tool method implementation
rg -nP --type=py -A30 'def get_tool\s*\(' src/fastmcp/tools/tool_manager.py
# Search for filtering logic related to exclude_tags or disabled
rg -nP --type=py 'exclude_tags|disabled|_should_enable' src/fastmcp/tools/tool_manager.py
Length of output: 1191
This is revealing. The get_tools() method explicitly returns "unfiltered" tools. Let me find where filtering actually happens.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for where exclude_tags filtering is actually applied
rg -nP --type=py 'exclude_tags' src/
# Search for list_tools implementation (this is what's called by MCP protocol)
rg -nP --type=py -A20 'async def list_tools' src/
# Search for _should_enable or similar filtering checks
rg -nP --type=py '_should_enable|_is_component_enabled' src/
Length of output: 6320
I found the filtering logic. Let me check if filtering is applied in the call_tool path on the server.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context around line 1114 to understand the call_tool filtering
rg -nP --type=py -B5 -A15 '^\s*if not self._should_enable_component\(tool\)' src/fastmcp/server/server.py | head -50
# Find the actual call_tool method in server.py
rg -nP --type=py -A30 'def call_tool\s*\(' src/fastmcp/server/server.py | head -60
Length of output: 2081
Now I need to verify if this is intentional behavior or if the test actually covers this scenario.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the complete call_tool method in server.py to see the full picture
rg -nP --type=py -B5 -A50 'async def call_tool\s*\(' src/fastmcp/server/server.py | head -80
# Look at the bulk tool caller test to see if it attempts to call filtered tools
cat tests/server/middleware/test_bulk_tool_caller.py | grep -A 50 "test_bulk_tools_bypass_filtering"
Length of output: 1788
Add test verifying bulk tools respect filtering when calling tools.
The current test confirms bulk tools bypass list filtering, but doesn't verify whether they can invoke filtered-out tools. The server's normal call_tool path applies filtering via _should_enable_component() (line 1127 in server.py), but bulk tools call _tool_manager.call_tool() directly (lines 45, 109 in bulk_tool_caller.py), bypassing this check. This is a security gap if tools are excluded for authorization/security reasons.
Add a test case that attempts to call a filtered-out tool via call_tool_bulk and verify it raises an appropriate error (e.g., NotFoundError or ToolError).
🤖 Prompt for AI Agents
In tests/server/middleware/test_bulk_tool_caller.py around lines 306 to 327, add
a new test that ensures bulk tool invocation respects server filtering by
attempting to call a tool tagged with an excluded tag via the bulk caller and
asserting it raises the expected error; specifically, after registering the
"multiply" tool tagged {"math"} and creating Client with exclude_tags={"math"},
use client.call_tool_bulk (or call_tools_bulk) to invoke "multiply" and assert
the call fails with the same error type the server uses for disallowed/missing
tools (e.g., NotFoundError or ToolError) and a suitable message, mirroring the
normal call_tool path behavior so bulk callers do not bypass
_should_enable_component().
Summary
Converts the BulkToolCaller from an MCPMixin-based implementation to a ToolInjectionMiddleware, following the pattern established in #2142. This simplifies usage and aligns with other tool injection middleware like PromptToolMiddleware and ResourceToolMiddleware.
Changes
BulkToolCallerMiddlewareextendingToolInjectionMiddlewarebulk_tool_caller_types.pyto avoid circular importsBulkToolCallerwith clear migration guidanceUsage
New approach:
Old approach (deprecated):
Closes #2262
🤖 Generated with Claude Code) | View branch | [View job run](https://github.com/jlowin/fastmcp/actions/runs/18819279688
Summary by CodeRabbit
New Features
Deprecations
Tests