-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add transform system for modifying components in provider chains #2836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tool transformations were previously scattered across LocalProvider,
ProxyProvider, and MCPConfig. This consolidates all transformation
logic into TransformingProvider via with_transforms(tool_transforms={...}).
- Add tool_transforms parameter to TransformingProvider
- Add tool_transforms to Provider.with_transforms()
- Remove transformation storage from LocalProvider and ProxyProvider
- Remove add_tool_transformation() and remove_tool_transformation() from FastMCP
- Add tool_transforms parameter to factory methods (from_openapi, from_fastapi, create_proxy)
- Update tests to use new patterns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbc0a3b35d
ℹ️ 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".
| tool = await self._wrapped.get_tool(original) | ||
| if tool: | ||
| return tool.model_copy(update={"name": name}) | ||
| # Apply transforms using the pre_transform_name (the key in tool_transforms) | ||
| return self._apply_tool_transform(tool, pre_transform_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject calls to tools by their pre-transform name
When a tool is renamed via tool_transforms (e.g., {"add": ToolTransformConfig(name="add_transformed")}), get_tool("add") still returns a transformed tool because the pre-transform name is passed into _apply_tool_transform. That means callers can invoke the old name even though list_tools only advertises the new name, which is a behavior regression from the prior implementation where old names were not resolvable. This can leak renamed tools under both names and makes name-based access control inconsistent. Consider returning None when the requested name doesn't match the transformed name (or only applying transforms after verifying the requested name is valid).
Useful? React with 👍 / 👎.
|
Warning Rate limit exceeded@jlowin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughAdds a middleware-style Transform system and applies it across server and provider layers: introduces a Transform base and concrete transforms (Namespace, ToolTransform, Visibility); replaces per-provider Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/fastmcp/server/providers/base.py (1)
71-134: Fix docstring example:ToolTransformConfig(arguments=...)(notargs=...).The example appears inconsistent with
ToolTransformConfig.arguments(persrc/fastmcp/tools/tool_transform.py).Proposed doc fix
# Apply schema transforms to modify tool arguments provider = MyProvider().with_transforms( - tool_transforms={"my_tool": ToolTransformConfig( - args={"old_arg": ArgTransform(name="new_arg")} - )} + tool_transforms={ + "my_tool": ToolTransformConfig( + arguments={"old_arg": ArgTransformConfig(name="new_arg")} + ) + } )src/fastmcp/server/providers/transforming.py (2)
43-62: Fix docstring example:ToolTransformConfig(arguments=...)(notargs=...).Proposed doc fix
tool_renames={"verbose_tool_name": "short"}, - tool_transforms={"short": ToolTransformConfig( - args={"old_arg": ArgTransform(name="new_arg")} - )} + tool_transforms={ + "short": ToolTransformConfig( + arguments={"old_arg": ArgTransformConfig(name="new_arg")} + ) + } )
64-108: Add validation fortool_transformsrename collisions (duplicate final names).Right now
_tool_transforms_name_reversewill silently overwrite if two transforms rename to the sameconfig.name, andget_tool()becomes ambiguous if a transform renames to another transform’s key/final name.Proposed fix (validate unique final names + build reverse map safely)
class TransformingProvider(Provider): @@ def __init__( @@ self.tool_renames = tool_renames or {} self.tool_transforms = tool_transforms or {} @@ - # Build reverse mapping for tool_transforms that rename tools - # Maps: final_name -> pre_transform_name (the key in tool_transforms) - self._tool_transforms_name_reverse: dict[str, str] = {} - for pre_transform_name, config in self.tool_transforms.items(): - if config.name is not None: - self._tool_transforms_name_reverse[config.name] = pre_transform_name + # Build reverse mapping for tool_transforms (and validate unique final names) + # Maps: final_name -> pre_transform_name (the key in tool_transforms) + self._tool_transforms_name_reverse: dict[str, str] = {} + final_names: dict[str, str] = {} + for pre_transform_name, config in self.tool_transforms.items(): + final_name = config.name or pre_transform_name + if final_name in final_names and final_names[final_name] != pre_transform_name: + raise ValueError( + "tool_transforms produces duplicate final tool name " + f"{final_name!r}: both {final_names[final_name]!r} and {pre_transform_name!r}" + ) + final_names[final_name] = pre_transform_name + if config.name is not None: + self._tool_transforms_name_reverse[final_name] = pre_transform_name
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/server/providers/proxy/test_proxy_server.pyis excluded by none and included by nonetests/server/providers/test_local_provider.pyis excluded by none and included by nonetests/server/test_tool_transformation.pyis excluded by none and included by none
📒 Files selected for processing (8)
src/fastmcp/client/transports.pysrc/fastmcp/mcp_config.pysrc/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/filesystem.pysrc/fastmcp/server/providers/local_provider.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/providers/transforming.pysrc/fastmcp/server/server.py
💤 Files with no reviewable changes (1)
- src/fastmcp/server/providers/filesystem.py
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Python ≥ 3.10 with full type annotations required
Prioritize readable, understandable code - clarity over cleverness. Avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code implementation
Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Never use bare except - be specific with exception types
Files:
src/fastmcp/mcp_config.pysrc/fastmcp/server/providers/local_provider.pysrc/fastmcp/server/providers/transforming.pysrc/fastmcp/server/server.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/client/transports.pysrc/fastmcp/server/providers/base.py
🧠 Learnings (3)
📚 Learning: 2025-12-25T15:53:07.656Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T15:53:07.656Z
Learning: Applies to tests/**/*.py : Pass FastMCP servers directly to clients for testing without network complexity; only use HTTP transport when explicitly testing network features
Applied to files:
src/fastmcp/mcp_config.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/client/transports.py
📚 Learning: 2025-12-25T15:53:07.656Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T15:53:07.656Z
Learning: Applies to src/fastmcp/**/*.py : Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Applied to files:
src/fastmcp/mcp_config.pysrc/fastmcp/server/providers/proxy.py
📚 Learning: 2025-12-25T15:53:07.656Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T15:53:07.656Z
Learning: Applies to src/fastmcp/**/*.py : Python ≥ 3.10 with full type annotations required
Applied to files:
src/fastmcp/server/providers/local_provider.pysrc/fastmcp/server/providers/proxy.py
🧬 Code graph analysis (4)
src/fastmcp/server/providers/transforming.py (3)
src/fastmcp/tools/tool_transform.py (2)
ToolTransformConfig(889-927)apply(916-927)src/fastmcp/server/providers/base.py (1)
Provider(47-360)src/fastmcp/tools/tool.py (5)
tool(775-775)tool(779-790)tool(794-806)tool(809-944)Tool(141-395)
src/fastmcp/server/server.py (3)
src/fastmcp/server/providers/base.py (2)
Provider(47-360)with_transforms(71-134)src/fastmcp/tools/tool_transform.py (1)
ToolTransformConfig(889-927)src/fastmcp/server/providers/proxy.py (1)
FastMCPProxy(623-667)
src/fastmcp/server/providers/proxy.py (3)
src/fastmcp/tools/tool_transform.py (1)
ToolTransformConfig(889-927)src/fastmcp/server/providers/base.py (2)
Provider(47-360)with_transforms(71-134)src/fastmcp/server/server.py (1)
add_provider(708-718)
src/fastmcp/server/providers/base.py (1)
src/fastmcp/tools/tool_transform.py (1)
ToolTransformConfig(889-927)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
🔇 Additional comments (10)
src/fastmcp/client/transports.py (1)
1009-1045: Correct wiring for renamed kwarg (tool_transforms).This looks like the right propagation point now that
create_proxy(...)acceptstool_transforms.src/fastmcp/mcp_config.py (1)
92-117: Kwarg rename in_to_server_and_underlying_transport()is consistent.src/fastmcp/server/providers/base.py (1)
33-45: TYPE_CHECKING import for ToolTransformConfig is a good fit here.src/fastmcp/server/providers/local_provider.py (1)
217-235: Simplified tool access path (visibility preserved).src/fastmcp/server/providers/transforming.py (1)
186-220: Transform application order matches the documented intent (namespace/rename → schema transforms).src/fastmcp/server/providers/proxy.py (2)
477-490: ProxyProvider no longer mutates tools; good separation of concerns.
642-668: No action needed—backend dispatch is correctly preserved.ProxyTool implements a
_backend_namefield to track the original tool name. Whenmodel_copy()is called with a name change (which happens during transformation), it captures the original name in_backend_nameon the first rename. Later,ProxyTool.run()dispatches viaclient.call_tool_mcp(name=self._backend_name or self.name, ...), ensuring the backend receives the original tool name regardless of any client-side renames.The transformation pipeline correctly passes the renamed ProxyTool (with
_backend_nameset) as theparent_toolreference in TransformedTool, so schema transformations applied viaToolTransformConfig(name=...)do not interfere with backend dispatch.src/fastmcp/server/server.py (3)
252-268: Back-compat wrapping of LocalProvider via TransformingProvider looks correct.
2424-2534: Factory methods applyingtool_transformsviawith_transforms()is the right direction.
2588-2644: No remaining call sites use the old/removed APIs. Verified no usage oftool_transformations=(old kwarg name) oradd_tool_transformation/remove_tool_transformation(removed methods) throughout the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/fastmcp/server/providers/transforming.py (1)
205-224: Consider adding inline comments to clarify the multi-step reverse lookup.The logic correctly implements reverse transformation and prevents access by pre-transform names, but the multi-step process (lines 207-212) could benefit from additional inline comments explaining the transformation chain. The check at line 222 is well-documented, but the earlier steps would be clearer with brief explanatory comments.
📝 Suggested inline comments for clarity
async def get_tool(self, name: str) -> Tool | None: """Get tool by transformed name.""" - # First check if name was transformed by tool_transforms (e.g., renamed) - # If so, get the pre-transform name (the key in tool_transforms) + # Step 1: If the requested name was renamed by tool_transforms, + # resolve it back to the pre-transform name (the key in tool_transforms dict). + # Otherwise, use the requested name as-is. pre_transform_name = self._tool_transforms_name_reverse.get(name, name) - # Now reverse the namespace/rename transformation + # Step 2: Reverse the namespace/rename transformation to get the original + # tool name from the wrapped provider. original = self._reverse_tool_name(pre_transform_name) if original is None: return None + # Step 3: Fetch the original tool tool = await self._wrapped.get_tool(original) if tool: # Apply transforms using the pre_transform_name (the key in tool_transforms) transformed = self._apply_tool_transform(tool, pre_transform_name) # Only return if requested name matches the final transformed name # This prevents accessing tools by their pre-transform name if transformed.name == name: return transformed return None
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/providers/test_local_provider.pyis excluded by none and included by none
📒 Files selected for processing (1)
src/fastmcp/server/providers/transforming.py
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Python ≥ 3.10 with full type annotations required
Prioritize readable, understandable code - clarity over cleverness. Avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code implementation
Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/providers/transforming.py
🧬 Code graph analysis (1)
src/fastmcp/server/providers/transforming.py (2)
src/fastmcp/tools/tool_transform.py (2)
ToolTransformConfig(889-927)apply(916-927)src/fastmcp/server/providers/base.py (1)
Provider(47-360)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/server/providers/transforming.py (3)
19-19: LGTM: Clean integration of tool_transforms parameter.The import, parameter definition, and storage follow consistent patterns with the existing tool_renames implementation. Type annotations are correct and follow Python >= 3.10 syntax.
Also applies to: 70-70, 87-87
186-195: LGTM: Well-structured transformation helper.The two-step transformation approach is clear and correct: first apply namespace/rename (line 189), then apply schema transforms (lines 192-193). This properly implements the transformation order documented in the class docstring.
197-203: LGTM: Consistent transformation application across methods.Both
list_toolsandget_taskscorrectly apply_apply_tool_transformusing the transformed tool names. The implementation is clean and maintains consistency with the overall transformation strategy.Also applies to: 302-333
- Validate duplicate target names in tool_transforms raise ValueError - Fix docstring examples to use arguments/ArgTransformConfig (not args/ArgTransform) - Add test for collision validation
- Add AggregateProvider to present multiple providers as one - Add _get_root_provider() to apply server-level transforms uniformly - Fix _docket_lifespan to use root provider (ensures renamed tools register with correct keys for background execution) - Add tool_transforms kwarg to __init__ (non-deprecated) - Add add_tool_transform(), remove_tool_transform(), tool_transforms property - Deprecate old API names (tool_transformations, add_tool_transformation, etc.) - Update tests to use new API
Test Failure AnalysisSummary: Test timeout in Root Cause: The test creates an OpenAPI server and attempts to call a tool with an httpx.AsyncClient configured with The timeout appears to be caused by slow DNS resolution or network connection attempts in the specific CI environment for Python 3.13. The test completed in:
The only change between the passing and failing run was a merge from main (commit 766641a), which appears to be unrelated to this functionality. Suggested Solution:
Detailed AnalysisEvidence of Flakiness
Why This HappensThe test uses
Log Excerpt from Local RunThe test passes correctly - it just needs more time in some CI environments. Related Files
Updated analysis for workflow run 20941415903 (commit 3721dde) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/fastmcp/server/providers/transforming.py (1)
22-23: Remove emptyTYPE_CHECKINGblock.This block is empty and serves no purpose since
ToolTransformConfigis now imported directly at line 19.♻️ Suggested fix
-if TYPE_CHECKING: - passsrc/fastmcp/server/server.py (1)
771-780: Consider caching the root provider for repeated calls.
_get_root_provider()creates a newAggregateProvider(and potentiallyTransformingProvider) on every call. During high-frequency operations (e.g., multipleget_toolcalls), this creates unnecessary object allocations.However, caching would require invalidation when
_providersor_tool_transformschange. The current approach is simpler and correct. This is a minor optimization opportunity if profiling shows it's a bottleneck.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/server/providers/test_local_provider.pyis excluded by none and included by nonetests/server/test_tool_transformation.pyis excluded by none and included by none
📒 Files selected for processing (4)
src/fastmcp/server/providers/aggregate.pysrc/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/transforming.pysrc/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Python ≥ 3.10 with full type annotations required
Prioritize readable, understandable code - clarity over cleverness. Avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code implementation
Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/providers/aggregate.pysrc/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/transforming.pysrc/fastmcp/server/server.py
🧠 Learnings (1)
📚 Learning: 2025-12-25T15:53:07.656Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T15:53:07.656Z
Learning: Applies to src/fastmcp/**/*.py : Python ≥ 3.10 with full type annotations required
Applied to files:
src/fastmcp/server/server.py
🧬 Code graph analysis (3)
src/fastmcp/server/providers/aggregate.py (5)
src/fastmcp/prompts/prompt.py (1)
Prompt(195-390)src/fastmcp/resources/resource.py (1)
Resource(213-402)src/fastmcp/resources/template.py (1)
ResourceTemplate(100-308)src/fastmcp/server/providers/base.py (1)
Provider(47-360)src/fastmcp/tools/tool.py (1)
Tool(144-398)
src/fastmcp/server/providers/base.py (2)
src/fastmcp/tools/tool_transform.py (1)
ToolTransformConfig(889-927)src/fastmcp/server/server.py (1)
tool_transforms(783-788)
src/fastmcp/server/server.py (10)
src/fastmcp/server/context.py (4)
fastmcp(169-174)list_resources(301-307)list_prompts(309-315)get_prompt(317-334)src/fastmcp/server/providers/aggregate.py (10)
get_tasks(132-135)list_tools(48-51)get_tool(53-59)list_resources(65-68)get_resource(70-76)list_resource_templates(82-87)get_resource_template(89-95)list_prompts(101-104)get_prompt(106-112)get_component(118-126)src/fastmcp/tools/tool_transform.py (1)
ToolTransformConfig(889-927)src/fastmcp/server/providers/base.py (13)
get_tasks(262-283)Provider(47-360)with_transforms(71-134)list_tools(153-158)_is_component_enabled(349-360)get_tool(160-170)list_resources(172-177)get_resource(179-189)list_resource_templates(191-196)get_resource_template(198-212)list_prompts(214-219)get_prompt(221-231)get_component(233-256)src/fastmcp/server/providers/local_provider.py (17)
get_tasks(301-308)list_tools(217-223)tool(315-331)tool(334-350)tool(356-502)get_tool(225-234)list_resources(236-242)resource(504-583)get_resource(244-249)list_resource_templates(251-257)get_resource_template(259-268)list_prompts(270-276)prompt(586-598)prompt(601-613)prompt(615-703)get_prompt(278-283)get_component(285-295)src/fastmcp/server/providers/openapi/provider.py (8)
get_tasks(382-384)list_tools(351-353)get_tool(355-357)list_resources(359-361)get_resource(363-365)list_resource_templates(367-369)get_resource_template(371-376)list_prompts(378-380)src/fastmcp/server/providers/filesystem.py (8)
list_tools(176-179)get_tool(181-184)list_resources(186-189)get_resource(191-194)list_resource_templates(196-199)get_resource_template(201-204)list_prompts(206-209)get_prompt(211-214)src/fastmcp/resources/resource.py (3)
key(373-375)Resource(213-402)resource(501-627)src/fastmcp/resources/template.py (2)
key(277-279)ResourceTemplate(100-308)src/fastmcp/server/middleware/tool_injection.py (3)
list_resources(88-90)list_prompts(54-56)get_prompt(64-72)
🪛 GitHub Actions: Run static analysis
src/fastmcp/server/providers/aggregate.py
[error] 145-145: ruff-format failed: pre-commit hook reformatted the file. All changes made by hooks present. Re-run the formatter or commit the changes. Process completed with exit code 1.
🪛 Ruff (0.14.10)
src/fastmcp/server/providers/transforming.py
109-113: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/server.py
270-273: Avoid specifying long messages outside the exception class
(TRY003)
965-965: Avoid specifying long messages outside the exception class
(TRY003)
1016-1016: Avoid specifying long messages outside the exception class
(TRY003)
1069-1069: Avoid specifying long messages outside the exception class
(TRY003)
1117-1117: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (17)
src/fastmcp/server/providers/base.py (2)
33-44: LGTM!Correct use of
TYPE_CHECKINGto importToolTransformConfigonly for static type checking, avoiding potential circular import issues at runtime.
71-134: LGTM!The
tool_transformsparameter is well-integrated intowith_transforms(). The docstring clearly explains the transformation order (applied after namespace/rename), and the implementation correctly forwards the parameter toTransformingProvider.One minor note: the docstring example references
ArgTransformConfigwhich users would need to import separately, but this is acceptable as it's illustrative.src/fastmcp/server/providers/transforming.py (4)
102-114: LGTM! Validation prevents duplicate target names.The reverse mapping construction with collision detection ensures
tool_transformsthat rename multiple tools to the same target name are caught early with a clear error message.
193-202: LGTM!The
_apply_tool_transformhelper cleanly separates the namespace/rename step from the schema transform step. The order (namespace/rename first, then schema transform) matches the documented behavior.
212-231: Verify edge case: tool_transforms without a name rename.The logic assumes that if a tool is in
tool_transformsbutconfig.nameisNone, the tool won't appear in_tool_transforms_name_reverse. This is correct, but worth noting:
- If
tool_transforms={"foo": ToolTransformConfig(description="new desc")}(no name change),get_tool("foo")should still work since_tool_transforms_name_reverse.get("foo", "foo")returns"foo".- The final check
if transformed.name == nameensures correctness even in complex scenarios.The implementation handles this correctly.
309-316: LGTM!Background task registration now correctly applies both namespace/rename and schema transforms to
Toolcomponents, ensuring Docket workers receive the transformed tool definitions.src/fastmcp/server/providers/aggregate.py (2)
22-59: LGTM!The
AggregateProviderimplementation is clean and follows the established provider patterns. The combination of parallellist_*operations (usingasyncio.gather) and sequentialget_*lookups (first non-None wins) correctly implements the documented provider semantics.
141-147: LGTM! Proper lifespan composition.Using
AsyncExitStackto manage multiple provider lifespans ensures proper cleanup even if one provider's lifespan fails during teardown.src/fastmcp/server/server.py (9)
211-214: LGTM!Clean parameter additions with the deprecated
tool_transformationsalias properly separated. The type annotationMapping[str, ToolTransformConfig]is appropriately flexible for input.
260-279: LGTM! Proper deprecation handling.The deprecation logic correctly:
- Warns when using the deprecated parameter
- Prevents using both parameters simultaneously
- Falls back to the deprecated value when
tool_transformsis not provided
782-788: LGTM!The
tool_transformsproperty correctly returns a defensive copy to prevent external mutation of internal state.
790-823: LGTM!The
add_tool_transformandremove_tool_transformmethods provide a clean runtime API for managing transforms. The docstring example clearly demonstrates usage.
825-846: LGTM! Proper deprecation aliases.The deprecated methods correctly delegate to the new APIs while emitting deprecation warnings when
fastmcp.settings.deprecation_warningsis enabled.
500-511: LGTM!Task collection at startup now correctly uses the root provider to ensure server-level transforms are applied to task-eligible components before Docket registration.
946-954: LGTM!The refactored
get_toolsimplementation correctly:
- Fetches tools via the root provider (with transforms applied)
- Deduplicates by key (first wins)
- Filters by visibility
2463-2499: LGTM!The
from_openapifactory correctly acceptstool_transformsand applies them viawith_transforms()when provided, maintaining consistency with the provider wrapping pattern.
2616-2671: LGTM!The
create_proxyfunction correctly propagatestool_transformstoFastMCPProxy, enabling schema modifications on proxied tools.
| @asynccontextmanager | ||
| async def lifespan(self) -> AsyncIterator[None]: | ||
| """Combine lifespans of all providers.""" | ||
| async with AsyncExitStack() as stack: | ||
| for provider in self._providers: | ||
| await stack.enter_async_context(provider.lifespan()) | ||
| yield | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting to resolve pipeline failure.
The pipeline indicates ruff-format failed on this file. The file appears to be missing a trailing newline after line 147.
🔧 Suggested fix
Ensure the file ends with a single newline character after line 147:
await stack.enter_async_context(provider.lifespan())
yield
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @asynccontextmanager | |
| async def lifespan(self) -> AsyncIterator[None]: | |
| """Combine lifespans of all providers.""" | |
| async with AsyncExitStack() as stack: | |
| for provider in self._providers: | |
| await stack.enter_async_context(provider.lifespan()) | |
| yield | |
| @asynccontextmanager | |
| async def lifespan(self) -> AsyncIterator[None]: | |
| """Combine lifespans of all providers.""" | |
| async with AsyncExitStack() as stack: | |
| for provider in self._providers: | |
| await stack.enter_async_context(provider.lifespan()) | |
| yield |
🧰 Tools
🪛 GitHub Actions: Run static analysis
[error] 145-145: ruff-format failed: pre-commit hook reformatted the file. All changes made by hooks present. Re-run the formatter or commit the changes. Process completed with exit code 1.
Test Failure AnalysisSummary: The test Root Cause: During the refactoring, the error logging level in Suggested Solution: Change the logging level back to ERROR in # In _collect_list_results() at line 55
logger.error(
f"Error during {operation} from provider "
f"{self._providers[i]}: {result}"
)
# In _get_first_result() at line 71 (for non-NotFoundError exceptions)
logger.error(
f"Error during {operation} from provider "
f"{self._providers[i]}: {result}"
)This restores the original behavior where provider connection failures are logged as errors, which makes sense since they represent actual operational problems that users should be aware of. Detailed AnalysisWhat ChangedOld code (main branch) in if isinstance(result, BaseException):
provider = self._providers[i]
logger.exception(f"Error listing tools from provider {provider}")
# ...continue processing other providers
New code (PR branch) in if isinstance(result, BaseException):
logger.debug(
f"Error during {operation} from provider "
f"{self._providers[i]}: {result}"
)
Test ExpectationsThe test error_messages = [
record.message for record in caplog.records if record.levelname == "ERROR"
]
assert any("Error listing tools from provider" in msg for msg in error_messages)
assert any("Error listing resources from provider" in msg for msg in error_messages)
assert any("Error listing prompts from provider" in msg for msg in error_messages)Current BehaviorThe logs show DEBUG-level messages are being created: But the test looks for ERROR-level logs, so the assertion fails. Related Files
Updated analysis reflecting latest code changes (now has error handling with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/fastmcp/server/providers/aggregate.py (2)
38-45: Consider validating non-empty providers list.While an empty providers list would functionally work (returning empty results), it may indicate a configuration error. Consider adding validation or at minimum documenting the expected behavior.
Optional validation addition
def __init__(self, providers: Sequence[Provider]) -> None: """Initialize with a sequence of providers. Args: - providers: The providers to aggregate. Queried in order for lookups. + providers: The providers to aggregate. Queried in order for lookups. + May be empty, in which case all operations return empty results. """ super().__init__() self._providers = list(providers)Or add explicit validation if empty is unexpected:
def __init__(self, providers: Sequence[Provider]) -> None: """Initialize with a sequence of providers. Args: providers: The providers to aggregate. Queried in order for lookups. """ super().__init__() + if not providers: + raise ValueError("AggregateProvider requires at least one provider") self._providers = list(providers)
76-86: Consider more specific exception types for graceful degradation.The broad
Exceptioncatching in allget_*methods implements graceful degradation as documented, but per coding guidelines, more specific exception types are preferred. Consider catching specific exceptions that indicate provider failures vs. programming errors.For example, you might want to re-raise certain critical exceptions:
Example: More granular exception handling
async def get_tool(self, name: str) -> Tool | None: """Get tool by name from first provider that has it.""" for provider in self._providers: try: tool = await provider.get_tool(name) if tool is not None: return tool + except (asyncio.CancelledError, KeyboardInterrupt): + # Don't suppress cancellation or interrupts + raise except Exception as e: logger.warning(f"Error getting tool {name!r} from {provider}: {e}") continue return NoneAlternatively, document why broad catching is intentional:
async def get_tool(self, name: str) -> Tool | None: """Get tool by name from first provider that has it.""" for provider in self._providers: try: tool = await provider.get_tool(name) if tool is not None: return tool - except Exception as e: + except Exception as e: # Broad catch for graceful degradation logger.warning(f"Error getting tool {name!r} from {provider}: {e}") continue return NoneApply similar patterns to other
get_*methods.As per coding guidelines, specific exception types are preferred over broad catches.
Also applies to: 99-109, 123-135, 148-158, 164-176
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/server/providers/aggregate.py
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Python ≥ 3.10 with full type annotations required
Prioritize readable, understandable code - clarity over cleverness. Avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code implementation
Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/providers/aggregate.py
🪛 Ruff (0.14.10)
src/fastmcp/server/providers/aggregate.py
83-83: Do not catch blind exception: Exception
(BLE001)
106-106: Do not catch blind exception: Exception
(BLE001)
130-130: Do not catch blind exception: Exception
(BLE001)
155-155: Do not catch blind exception: Exception
(BLE001)
173-173: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (5)
src/fastmcp/server/providers/aggregate.py (5)
1-26: LGTM! Clean module structure.The imports are well-organized, the module docstring clearly describes the purpose, and the generic TypeVar setup for _collect_results is appropriate.
47-60: LGTM! Well-designed helper for graceful degradation.The
_collect_resultsmethod correctly handles results fromasyncio.gather(..., return_exceptions=True), logging errors while collecting successful results. The indexing is safe sinceenumerate(results)provides valid indices.
69-74: LGTM! Excellent use of parallel aggregation.All
list_*methods andget_tasksuseasyncio.gatherwithreturn_exceptions=Truefor optimal parallel execution while maintaining graceful degradation. The consistent pattern enhances maintainability.Also applies to: 92-97, 115-121, 141-146, 182-187
193-199: LGTM! Proper lifespan composition.The use of
AsyncExitStackcorrectly composes provider lifespans with proper cleanup semantics. If any provider's lifespan entry fails, the stack ensures already-entered providers are cleaned up in reverse order.
28-36: Well-designed aggregation pattern.The AggregateProvider successfully implements a composite pattern with clear semantics: parallel aggregation for list operations and first-match for get operations. The graceful degradation approach aligns with the stated design goals and enables robust multi-provider composition.
Also applies to: 62-63
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/fastmcp/server/providers/aggregate.py (2)
39-46: Consider documenting behavior with empty providers.The implementation correctly handles an empty
providerssequence (all operations would return empty results), but this edge case isn't documented. Consider adding a note about the expected behavior when no providers are supplied.📝 Suggested documentation enhancement
def __init__(self, providers: Sequence[Provider]) -> None: """Initialize with a sequence of providers. Args: - providers: The providers to aggregate. Queried in order for lookups. + providers: The providers to aggregate. Queried in order for lookups. + If empty, all operations return empty sequences or None. """
48-78: Consider lazy evaluation for debug log messages.The f-strings in debug logs (lines 55-57, 71-73) are always evaluated, even when debug logging is disabled. For better performance, consider using lazy formatting or conditional logging.
⚡ Optional performance improvement
if isinstance(result, BaseException): - logger.debug( - f"Error during {operation} from provider " - f"{self._providers[i]}: {result}" - ) + logger.debug( + "Error during %s from provider %s: %s", + operation, + self._providers[i], + result, + )Apply similar changes to lines 71-73.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/fastmcp/server/providers/aggregate.py
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Python ≥ 3.10 with full type annotations required
Prioritize readable, understandable code - clarity over cleverness. Avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code implementation
Be intentional about re-exports - don't blindly re-export everything to parent namespaces. Core types defining a module's purpose should be exported. Specialized features can live in submodules. Only re-export to fastmcp.* for most fundamental types
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/providers/aggregate.py
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (9)
src/fastmcp/server/providers/aggregate.py (9)
1-26: LGTM! Clear module structure.The module docstring effectively explains the purpose, and all imports are relevant to the implementation.
80-81: LGTM! Clean representation.
87-99: LGTM! Consistent aggregation pattern.The parallel querying of all providers in
get_toolis slightly inefficient (all providers are queried even though only the first non-None result is used), but this trades performance for implementation simplicity and is acceptable.
105-117: LGTM! Consistent implementation.
123-137: LGTM! Consistent with other entity types.
143-155: LGTM! Maintains consistent pattern.
161-168: LGTM! Correct type union and consistent pattern.
174-179: LGTM! Follows the list operation pattern correctly.
185-191: LGTM! Correct lifespan aggregation pattern.The use of
AsyncExitStackensures proper LIFO cleanup of provider lifespans, and will correctly handle cleanup if any provider's lifespan entry fails.
Replaces the ad-hoc transformation system with a unified Transform abstraction using the same call_next pattern as server middleware. Key changes: - New src/fastmcp/server/transforms/ module with Transform base class - Namespace, ToolTransform, Visibility all implement the same interface - Transforms compose via functools.partial chain building - Visibility is now just the first transform in provider._transforms - Server-level transforms apply after provider aggregation - Task registration now applies full transform chain Removes TransformingProvider, _BoundTransform, ComponentSource protocol. User-facing API unchanged: mount(), add_transform(), enable/disable all work as before.
Test Failure AnalysisSummary: Static analysis failed due to import ordering violations and file size limit violations in the transform consolidation PR. Root Cause:
Suggested Solution:
Detailed AnalysisImport Ordering IssueThe file has this structure: # Lines 1-25: Docstring and standard imports
from __future__ import annotations
from collections.abc import Awaitable, Callable, Sequence
from typing import TYPE_CHECKING
# Lines 32-184: Type aliases and Transform class definition
ListToolsNext = Callable[[], Awaitable[Sequence[Tool]]]
# ... other type aliases ...
class Transform:
# ... class definition ...
# Lines 186-189: Re-exports (THIS IS THE PROBLEM)
# Re-export built-in transforms
from fastmcp.server.transforms.namespace import Namespace
from fastmcp.server.transforms.tool_transform import ToolTransform
from fastmcp.server.transforms.visibility import VisibilityRuff's E402 rule forbids module-level imports after non-import code. The imports at lines 187-189 come after the class definition. loq Pre-commit HookThe Related Files
|
Test Failure AnalysisSummary: Static analysis failed due to import ordering violations and file size limit violations in the transform consolidation PR. Root Cause:
Suggested Solution:
Detailed AnalysisImport Ordering IssueThe file has this structure: # Lines 1-25: Docstring and standard imports
from __future__ import annotations
from collections.abc import Awaitable, Callable, Sequence
from typing import TYPE_CHECKING
# Lines 32-184: Type aliases and Transform class definition
ListToolsNext = Callable[[], Awaitable[Sequence[Tool]]]
# ... other type aliases ...
class Transform:
# ... class definition ...
# Lines 186-189: Re-exports (THIS IS THE PROBLEM)
# Re-export built-in transforms
from fastmcp.server.transforms.namespace import Namespace
from fastmcp.server.transforms.tool_transform import ToolTransform
from fastmcp.server.transforms.visibility import VisibilityRuff's E402 rule forbids module-level imports after non-import code. The imports at lines 187-189 come after the class definition. loq Pre-commit HookThe Related Files
|
New docs/servers/providers/transforms.mdx covering: - Mental model for middleware-style transform pattern - Built-in transforms (Namespace, ToolTransform) - Server vs provider-level transforms and ordering - Tool modification (immediate vs deferred) - Custom transform creation New docs/servers/visibility.mdx covering: - Enable/disable API for runtime visibility control - Keys and tags for targeting components - Allowlist mode with only=True - Server vs provider visibility layering Updates existing docs to reference new pages and simplifies redundant content. Visibility is documented as a user feature, not as an implementation detail.
Test Failure AnalysisSummary: The Root Cause: This PR adds significant functionality to consolidate tool transformation logic, causing several files to grow beyond their configured limits. The Suggested Solution: Merge the latest git merge main
uv run loq baseline
git add loq.toml
git commit -m "Update loq.toml baseline for file size increases"Detailed AnalysisThe CI reported these violations (current lines > limit):
The most significant increases are in:
These increases align with the PR's goal of consolidating tool transformation logic into a unified system. Note: The Related Files
|
Test Failure AnalysisSummary: Static analysis failed because ruff made automatic formatting fixes that were not committed to the branch. Root Cause: The file
When the CI runs Suggested Solution: Run The specific changes needed:
Detailed AnalysisFrom the CI logs, ruff check and ruff format both made modifications: The diff shows: --- a/tests/deprecated/test_add_tool_transformation.py
+++ b/tests/deprecated/test_add_tool_transformation.py
@@ -2,8 +2,6 @@
import warnings
-import pytest
-
from fastmcp import FastMCP
from fastmcp.client import Client
from fastmcp.tools.tool_transform import ToolTransformConfig
@@ -76,9 +74,7 @@ class TestAddToolTransformationDeprecated:
warnings.simplefilter("always")
FastMCP(
"test",
- tool_transformations={
- "my_tool": ToolTransformConfig(name="renamed")
- },
+ tool_transformations={"my_tool": ToolTransformConfig(name="renamed")},
)Related Files
Updated analysis for latest workflow run (20939932785) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fastmcp/server/server.py (1)
2376-2470: Major:mount()returnsNonebut docs/examples use its return value (mount.add_transform(...)).
Givendocs/servers/providers/transforms.mdxandsrc/fastmcp/server/transforms/__init__.pyexamples, either:
- Change
mount()to return the created provider/mount handle, or- Update all examples/docs to not rely on the return value.
If you want option (1), this is the minimal change:
Proposed diff
def mount( @@ - ) -> None: + ) -> Provider: @@ self._providers.append(provider) + return provider
🧹 Nitpick comments (11)
src/fastmcp/contrib/component_manager/component_service.py (1)
19-52: Expose transforms access through a public API instead of accessing private_transformsattribute.The code accesses
provider._transformsat line 39, which is a private attribute (by Python convention). This tightly couples the function to the Provider's internal implementation. Consider adding a public method or property on theProviderclass (e.g.,get_transforms()or atransformsproperty) to access the transform list, allowing for future refactoring of the internal structure without breaking this code.src/fastmcp/server/transforms/__init__.py (1)
193-206: Confirm__all__here is intended to be a stable public surface.
You’re exporting the call-next type aliases as well as concrete transforms; that’s fine, but it’s a commitment. (If you want flexibility, consider keeping the aliases internal and only exportingTransform+ built-ins.) Based on learnings, be intentional about re-exports.src/fastmcp/server/providers/fastmcp_provider.py (3)
118-131: Improve the “unexpected CreateTaskResult” error with actionable context.
Right now the RuntimeError loses which tool and what was returned.Proposed diff
- if isinstance(result, mcp.types.CreateTaskResult): - raise RuntimeError( - "Unexpected CreateTaskResult from call_tool without task_meta" - ) + if isinstance(result, mcp.types.CreateTaskResult): + raise RuntimeError( + f"Unexpected {type(result).__name__} from call_tool() without task_meta " + f"for tool {self.name!r} (original={self._original_name!r})" + )
248-261: Same improvement for prompt rendering error context.Proposed diff
- if isinstance(result, mcp.types.CreateTaskResult): - raise RuntimeError( - "Unexpected CreateTaskResult from render_prompt without task_meta" - ) + if isinstance(result, mcp.types.CreateTaskResult): + raise RuntimeError( + f"Unexpected {type(result).__name__} from render_prompt() without task_meta " + f"for prompt {self.name!r} (original={self._original_name!r})" + )
520-571: Consider deduplicating transform-chain wiring (same pattern appears in multiple places).
This repeats the “base → partial(transform.list_*) chain” logic that also exists insrc/fastmcp/server/providers/base.pyandsrc/fastmcp/server/server.py. A small shared helper (e.g., “apply_list_transforms(transforms, list_fn)”) would cut maintenance risk.src/fastmcp/server/providers/aggregate.py (2)
48-61: Preserve tracebacks when logging provider exceptions.
Right now you log...: {result}which loses traceback context (very hard to debug intermittent provider failures).Proposed diff
def _collect_list_results( self, results: list[Sequence[T] | BaseException], operation: str ) -> list[T]: """Collect successful list results, logging any exceptions.""" collected: list[T] = [] for i, result in enumerate(results): if isinstance(result, BaseException): - logger.debug( - f"Error during {operation} from provider " - f"{self._providers[i]}: {result}" - ) + logger.debug( + "Error during %s from provider %r", + operation, + self._providers[i], + exc_info=(type(result), result, result.__traceback__), + ) continue collected.extend(result) return collected @@ def _get_first_result( self, results: list[T | None | BaseException], operation: str ) -> T | None: """Get first successful non-None result, logging non-NotFoundError exceptions.""" for i, result in enumerate(results): if isinstance(result, BaseException): # NotFoundError is expected - don't log it if not isinstance(result, NotFoundError): - logger.debug( - f"Error during {operation} from provider " - f"{self._providers[i]}: {result}" - ) + logger.debug( + "Error during %s from provider %r", + operation, + self._providers[i], + exc_info=(type(result), result, result.__traceback__), + ) continue if result is not None: return result return NoneAlso applies to: 63-78
95-102: Parallelget_*queries run all providers even when the first one succeeds.
That matches your “parallel lookups” goal, but it can create unnecessary load for expensive providers. If this becomes an issue, consider a “race + cancel pending” approach.Also applies to: 115-122, 135-142, 155-162
docs/servers/providers/transforms.mdx (1)
180-263: Add a small troubleshooting section for common transform failures.
Suggested bullets:
- “Tool not found after renaming” (reverse mapping in
get_tool)- “Visibility keys don’t match after transforms” (keys must match the server’s current names)
- “Hidden args + default_factory” gotchas
Based on learnings, include troubleshooting for likely failure points.
Also applies to: 344-405
src/fastmcp/server/transforms/visibility.py (1)
94-106: Consider isolating notification failures from enable/disable state changes.
Ifsend_notification_sync()throws, callers might see enable/disable fail even though internal state already mutated. A small try/except around each send (log debug) would make state changes more robust.src/fastmcp/server/providers/base.py (1)
296-363: Simplifyget_tasks()by using the existing_list_*methods (avoids duplicate chain logic).
Right nowget_tasks()manually reconstructs transform chains that already exist in_list_tools/_list_resources/....Proposed diff
async def get_tasks(self) -> Sequence[FastMCPComponent]: @@ - # Fetch all component types in parallel - results = await gather( - self.list_tools(), - self.list_resources(), - self.list_resource_templates(), - self.list_prompts(), - ) - tools = cast(Sequence[Tool], results[0]) - resources = cast(Sequence[Resource], results[1]) - templates = cast(Sequence[ResourceTemplate], results[2]) - prompts = cast(Sequence[Prompt], results[3]) - - # Apply provider's own transforms to components using the chain pattern - # For tasks, we need the fully-transformed names, so use the list_ chain - # Note: We build mini-chains for each component type - async def tools_base() -> Sequence[Tool]: - return tools - - async def resources_base() -> Sequence[Resource]: - return resources - - async def templates_base() -> Sequence[ResourceTemplate]: - return templates - - async def prompts_base() -> Sequence[Prompt]: - return prompts - - # Apply transforms in order (first is innermost) - tools_chain = tools_base - resources_chain = resources_base - templates_chain = templates_base - prompts_chain = prompts_base - - for transform in self._transforms: - tools_chain = partial(transform.list_tools, call_next=tools_chain) - resources_chain = partial( - transform.list_resources, call_next=resources_chain - ) - templates_chain = partial( - transform.list_resource_templates, call_next=templates_chain - ) - prompts_chain = partial(transform.list_prompts, call_next=prompts_chain) - - transformed_tools = await tools_chain() - transformed_resources = await resources_chain() - transformed_templates = await templates_chain() - transformed_prompts = await prompts_chain() + # Fetch fully-transformed component types in parallel (includes visibility) + tools, resources, templates, prompts = cast( + tuple[ + Sequence[Tool], + Sequence[Resource], + Sequence[ResourceTemplate], + Sequence[Prompt], + ], + tuple( + await gather( + self._list_tools(), + self._list_resources(), + self._list_resource_templates(), + self._list_prompts(), + ) + ), + ) return [ c for c in [ - *transformed_tools, - *transformed_resources, - *transformed_templates, - *transformed_prompts, + *tools, + *resources, + *templates, + *prompts, ] if c.task_config.supports_tasks() ]src/fastmcp/server/server.py (1)
774-877: Optional: factor_source_list_*/_source_get_*chain-building to reduce repetition.
A small internal helper like_apply_server_transforms_list(fn, method_name)/_apply_server_transforms_get(fn, method_name, arg)would make future transform expansion less error-prone.Also applies to: 878-932
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
AGENTS.mdis excluded by none and included by noneloq.tomlis excluded by none and included by nonetests/server/providers/proxy/test_proxy_server.pyis excluded by none and included by nonetests/server/providers/test_base_provider.pyis excluded by none and included by nonetests/server/providers/test_local_provider.pyis excluded by none and included by nonetests/server/providers/test_transforming_provider.pyis excluded by none and included by nonetests/server/test_mount.pyis excluded by none and included by nonetests/server/test_server.pyis excluded by none and included by nonetests/server/test_tool_transformation.pyis excluded by none and included by nonetests/utilities/test_visibility.pyis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (25)
docs/development/upgrade-guide.mdxdocs/development/v3-notes/v3-features.mdxdocs/docs.jsondocs/patterns/tool-transformation.mdxdocs/servers/middleware.mdxdocs/servers/providers/local.mdxdocs/servers/providers/mounting.mdxdocs/servers/providers/namespacing.mdxdocs/servers/providers/overview.mdxdocs/servers/providers/transforms.mdxdocs/servers/visibility.mdxsrc/fastmcp/client/transports.pysrc/fastmcp/contrib/component_manager/component_service.pysrc/fastmcp/mcp_config.pysrc/fastmcp/server/providers/__init__.pysrc/fastmcp/server/providers/aggregate.pysrc/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/fastmcp_provider.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/providers/transforming.pysrc/fastmcp/server/server.pysrc/fastmcp/server/transforms/__init__.pysrc/fastmcp/server/transforms/namespace.pysrc/fastmcp/server/transforms/tool_transform.pysrc/fastmcp/server/transforms/visibility.py
💤 Files with no reviewable changes (3)
- docs/patterns/tool-transformation.mdx
- src/fastmcp/server/providers/init.py
- src/fastmcp/server/providers/transforming.py
✅ Files skipped from review due to trivial changes (1)
- docs/servers/providers/mounting.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/fastmcp/mcp_config.py
- src/fastmcp/client/transports.py
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/providers/namespacing.mdxdocs/servers/providers/overview.mdxdocs/servers/middleware.mdxdocs/servers/providers/local.mdxdocs/development/upgrade-guide.mdxdocs/development/v3-notes/v3-features.mdxdocs/servers/visibility.mdxdocs/servers/providers/transforms.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns
Follow existing patterns and maintain consistency across the codebase
Files:
src/fastmcp/server/transforms/__init__.pysrc/fastmcp/contrib/component_manager/component_service.pysrc/fastmcp/server/transforms/tool_transform.pysrc/fastmcp/server/transforms/namespace.pysrc/fastmcp/server/providers/aggregate.pysrc/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/fastmcp_provider.pysrc/fastmcp/server/server.pysrc/fastmcp/server/transforms/visibility.pysrc/fastmcp/server/providers/proxy.py
src/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Be intentional about re-exports - don't blindly re-export everything to parent namespaces; only re-export fundamental types to fastmcp.*
Files:
src/fastmcp/server/transforms/__init__.py
🧠 Learnings (14)
📚 Learning: 2026-01-12T16:25:10.972Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:25:10.972Z
Learning: Applies to src/**/__init__.py : Be intentional about re-exports - don't blindly re-export everything to parent namespaces; only re-export fundamental types to fastmcp.*
Applied to files:
src/fastmcp/server/transforms/__init__.py
📚 Learning: 2026-01-12T16:24:54.978Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:54.978Z
Learning: Applies to src/tools/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Tools (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
docs/servers/middleware.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Maintain consistent terminology throughout all MDX documentation
Applied to files:
docs/servers/middleware.mdxdocs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Provide expected outcomes for each major step in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Cover complete request/response cycles in MDX API documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Provide dedicated troubleshooting sections for complex procedures in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Ensure content flows logically from basic concepts to advanced topics in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Add appropriate warnings for destructive or security-sensitive actions in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Anticipate common questions and address them proactively in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2025-11-26T21:52:08.947Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: docs/.cursor/rules/mintlify.mdc:0-0
Timestamp: 2025-11-26T21:52:08.947Z
Learning: Applies to docs/**/*.mdx : Include troubleshooting for likely failure points in MDX documentation
Applied to files:
docs/servers/providers/transforms.mdx
📚 Learning: 2026-01-12T16:25:10.972Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:25:10.972Z
Learning: Applies to docs/**/*.md : Documentation structure should use logical H2/H3 hierarchy with headers forming a navigation guide; prioritize user-focused sections with prose over code comments for important information
Applied to files:
docs/docs.json
📚 Learning: 2026-01-12T16:24:54.978Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:54.978Z
Learning: Maintain consistency across all four MCP object types (Tools, Resources, Resource Templates, and Prompts) when implementing similar features
Applied to files:
src/fastmcp/server/transforms/visibility.py
📚 Learning: 2026-01-12T16:25:10.972Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T16:25:10.972Z
Learning: When modifying MCP functionality (Tools, Resources, Resource Templates, or Prompts), changes typically need to be applied across all object types
Applied to files:
src/fastmcp/server/transforms/visibility.py
📚 Learning: 2026-01-12T16:24:54.978Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:54.978Z
Learning: Applies to src/prompts/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Prompts (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
src/fastmcp/server/transforms/visibility.py
🧬 Code graph analysis (6)
src/fastmcp/server/transforms/__init__.py (3)
src/fastmcp/server/transforms/namespace.py (9)
list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)Namespace(31-188)src/fastmcp/server/transforms/visibility.py (9)
list_tools(240-243)get_tool(245-250)list_resources(256-259)get_resource(261-268)list_resource_templates(274-279)get_resource_template(281-288)list_prompts(294-297)get_prompt(299-304)Visibility(42-304)src/fastmcp/server/transforms/tool_transform.py (3)
list_tools(63-73)get_tool(75-94)ToolTransform(15-94)
src/fastmcp/contrib/component_manager/component_service.py (2)
src/fastmcp/server/providers/base.py (1)
Provider(48-440)src/fastmcp/server/transforms/namespace.py (2)
_reverse_name(67-71)_reverse_uri(85-94)
src/fastmcp/server/providers/aggregate.py (5)
src/fastmcp/exceptions.py (1)
NotFoundError(34-35)src/fastmcp/server/providers/base.py (19)
base(108-109)base(127-128)base(139-140)base(151-152)base(163-164)base(175-176)base(187-188)base(199-200)Provider(48-440)list_tools(212-217)_list_tools(98-115)get_tool(219-229)_get_tool(117-134)list_resources(231-236)get_resource(238-248)list_resource_templates(250-255)get_resource_template(257-271)list_prompts(273-278)get_prompt(280-290)src/fastmcp/server/transforms/__init__.py (8)
list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)src/fastmcp/server/transforms/namespace.py (8)
list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)src/fastmcp/server/transforms/tool_transform.py (2)
list_tools(63-73)get_tool(75-94)
src/fastmcp/server/providers/base.py (6)
src/fastmcp/server/transforms/visibility.py (9)
Visibility(42-304)list_tools(240-243)get_tool(245-250)list_resources(256-259)get_resource(261-268)list_resource_templates(274-279)get_resource_template(281-288)list_prompts(294-297)get_prompt(299-304)src/fastmcp/server/transforms/__init__.py (9)
Transform(46-185)list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)src/fastmcp/server/transforms/namespace.py (8)
list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)src/fastmcp/server/providers/aggregate.py (8)
list_tools(87-93)get_tool(95-101)list_resources(107-113)get_resource(115-121)list_resource_templates(127-133)get_resource_template(135-141)list_prompts(147-153)get_prompt(155-161)src/fastmcp/server/providers/filesystem.py (8)
list_tools(176-179)get_tool(181-184)list_resources(186-189)get_resource(191-194)list_resource_templates(196-199)get_resource_template(201-204)list_prompts(206-209)get_prompt(211-214)src/fastmcp/server/providers/openapi/provider.py (7)
list_tools(351-353)get_tool(355-357)list_resources(359-361)get_resource(363-365)list_resource_templates(367-369)get_resource_template(371-376)list_prompts(378-380)
src/fastmcp/server/server.py (6)
src/fastmcp/server/providers/aggregate.py (9)
AggregateProvider(29-207)list_tools(87-93)get_tool(95-101)list_resources(107-113)get_resource(115-121)list_resource_templates(127-133)get_resource_template(135-141)list_prompts(147-153)get_prompt(155-161)src/fastmcp/server/transforms/namespace.py (9)
Namespace(31-188)list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)src/fastmcp/server/transforms/__init__.py (9)
Transform(46-185)list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)src/fastmcp/server/transforms/visibility.py (9)
Visibility(42-304)list_tools(240-243)get_tool(245-250)list_resources(256-259)get_resource(261-268)list_resource_templates(274-279)get_resource_template(281-288)list_prompts(294-297)get_prompt(299-304)src/fastmcp/server/providers/base.py (17)
base(108-109)base(127-128)base(139-140)base(151-152)base(163-164)base(175-176)base(187-188)base(199-200)list_tools(212-217)get_tool(219-229)list_resources(231-236)get_resource(238-248)list_resource_templates(250-255)get_resource_template(257-271)list_prompts(273-278)get_prompt(280-290)Provider(48-440)src/fastmcp/server/providers/local_provider.py (15)
list_tools(217-223)get_tool(225-234)list_resources(236-242)get_resource(244-249)list_resource_templates(251-257)get_resource_template(259-268)list_prompts(270-276)get_prompt(278-283)tool(315-331)tool(334-350)tool(356-502)resource(504-583)prompt(586-598)prompt(601-613)prompt(615-703)
src/fastmcp/server/transforms/visibility.py (2)
src/fastmcp/server/transforms/__init__.py (9)
Transform(46-185)list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)src/fastmcp/server/transforms/namespace.py (8)
list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)
🪛 Ruff (0.14.10)
src/fastmcp/server/transforms/__init__.py
189-189: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
190-190: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
191-191: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
src/fastmcp/server/transforms/tool_transform.py
51-54: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/providers/fastmcp_provider.py
127-129: Prefer TypeError exception for invalid type
(TRY004)
127-129: Avoid specifying long messages outside the exception class
(TRY003)
257-259: Prefer TypeError exception for invalid type
(TRY004)
257-259: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/server.py
1068-1068: Avoid specifying long messages outside the exception class
(TRY003)
1167-1167: Avoid specifying long messages outside the exception class
(TRY003)
1215-1215: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (29)
docs/servers/middleware.mdx (1)
327-327: LGTM!The link text and target correctly reflect the rename from "Tool Transformation" to "Transforms" and point to the new consolidated documentation page.
docs/development/upgrade-guide.mdx (1)
24-24: LGTM!The upgrade guide correctly reflects the migration from
TransformingProviderto the newTransformssystem withadd_transform()API.docs/servers/visibility.mdx (3)
1-11: LGTM!Proper frontmatter with title and description, appropriate version badge for v3.0.0 feature.
59-73: Clear component key documentation.The table format effectively documents the key format for each component type with practical examples.
186-234: Server vs Provider visibility well documented.The layered filtering concept is explained clearly with practical examples showing how visibility rules stack.
docs/servers/providers/local.mdx (2)
109-129: LGTM!The visibility section is appropriately simplified with a link to the central Visibility documentation page. This follows good documentation practices by avoiding duplication while still providing a quick example.
160-160: LGTM!Good addition of the visibility reference for standalone providers.
docs/docs.json (3)
108-108: LGTM!The
servers/providers/transformspage is correctly added to the Providers navigation group.
120-120: LGTM!The
servers/visibilitypage is correctly added to the Features navigation group.
562-570: LGTM!The redirects properly maintain backwards compatibility for the renamed/moved pages:
/servers/providers/namespacing→/servers/providers/transforms/patterns/tool-transformation→/servers/providers/transformsdocs/servers/providers/namespacing.mdx (1)
1-9: LGTM!The redirect page correctly points users to the new Transforms documentation while maintaining backward compatibility for existing links. Including both the
redirectfrontmatter field and an in-page notice is good practice.src/fastmcp/server/providers/proxy.py (2)
476-488: LGTM!The simplified
list_toolsimplementation is cleaner, directly returning a list comprehension ofProxyToolinstances. This aligns with the PR's goal of removing per-provider tool transformations in favor of the centralized transform pipeline.
660-661: LGTM!The simplified
FastMCPProxyinitialization now constructsProxyProviderwith only the client factory, delegating all transformation logic to the server-level transform chain as intended by this refactoring.src/fastmcp/contrib/component_manager/component_service.py (1)
71-81: LGTM!The logic correctly handles
FastMCPProviderwith transforms: it reverses through the transform chain when transforms exist, and returns the key unchanged for directFastMCPProviderinstances without transforms.docs/servers/providers/overview.mdx (2)
44-56: LGTM!The new Transforms section provides a clear and concise overview of the transform system. The table format effectively summarizes available transforms, and the text appropriately explains the automatic namespace creation during mounting.
73-80: LGTM!The Next Steps section is well-organized with clear, actionable links. The addition of Transforms and Visibility links appropriately guides users to the new documentation.
docs/development/v3-notes/v3-features.mdx (2)
94-97: LGTM!The updated mounting example correctly demonstrates the new pattern of explicitly creating a provider, adding transforms, and then mounting—replacing the previous chained method call approach.
100-120: Imports are correct, but the code example needs improvements per MDX guidelines.The import paths for
ToolTransformConfig,Namespace, andToolTransformare accurate and match patterns throughout the codebase. However, the code example uses a placeholder classSomeProvider()instead of concrete, realistic data. Per MDX documentation standards, provide a complete, executable example with a specific provider class, include expected output or verification steps, and add error handling for common failure points.src/fastmcp/server/transforms/tool_transform.py (3)
32-55: LGTM!The constructor correctly builds a reverse name mapping and validates that no two source tools map to the same target name. This defensive validation prevents subtle bugs where tool lookups would be ambiguous.
63-73: LGTM!The
list_toolsimplementation is clean and correct: it fetches tools from the next handler in the chain, applies transforms to matching tools, and passes through unmatched tools unchanged.
75-94: LGTM!The
get_toollogic correctly handles the bidirectional name mapping:
- Resolves the requested name to its original via the reverse mapping
- Fetches the original tool from the chain
- Applies the transform if applicable
- Returns the tool only if the final name matches the request
The final name check (lines 89-91, 94) ensures that requesting a non-existent transformed name doesn't accidentally return an untransformed tool.
src/fastmcp/server/transforms/namespace.py (4)
27-28: Consider edge case for non-standard URIs.The regex pattern
^([^:]+://)(.*?)$handles standardprotocol://pathURIs. For URIs that don't match this pattern (e.g.,urn:isbn:0451450523or malformed URIs),_transform_urireturns the URI unchanged while_reverse_urireturnsNone. This asymmetry is intentional per the design, but ensure upstream callers handle theNonecase from_reverse_uriappropriately.
100-115: LGTM!The tool namespace implementation is correct:
list_toolsprefixes all tool names, andget_toolreverses the namespace prefix before delegating, returning the tool with the namespaced name. The earlyNonereturn when the namespace doesn't match is the correct behavior for the transform chain.
145-167: LGTM!The resource template handling correctly transforms
uri_templatefields. Theget_resource_templatemethod appropriately re-applies the transform to the returned template'suri_templateto ensure consistency.
173-188: LGTM!The prompt namespace implementation mirrors the tool implementation correctly, applying the same name prefix pattern for consistency across component types.
src/fastmcp/server/transforms/visibility.py (1)
42-305: Nice: consistent filtering across tools/resources/templates/prompts.
The list/get implementations mirror each other cleanly and keep the “visibility is hierarchical” invariant. Based on learnings, consistency across all four MCP object types is important.src/fastmcp/server/providers/base.py (1)
66-93: Transform ordering matches intended semantics (visibility first/innermost).
Seeding_transformswithVisibility()and appending user transforms ensures provider-level visibility evaluates pre-renaming. Good.src/fastmcp/server/server.py (1)
757-932: Server-level ordering looks correct (visibility outermost so it evaluates transformed keys).
_get_all_transforms()returning[*_transforms, _visibility]combined with forward iteration produces the intended “server visibility sees transformed names” behavior.docs/servers/providers/transforms.mdx (1)
39-80: Docs/API mismatch:mount()is shown as returning a handle, but the code currently returnsNone.
Examples likemount = main.mount(...); mount.add_transform(...)won’t work unlessmount()returns the provider/mount handle. Align docs with implementation or updatemount()to return the created provider. As per coding guidelines, keep docs technically accurate and examples runnable.Also applies to: 295-301
⛔ Skipped due to learnings
Learnt from: CR Repo: jlowin/fastmcp PR: 0 File: docs/.cursor/rules/mintlify.mdc:0-0 Timestamp: 2025-11-26T21:52:08.947Z Learning: Applies to docs/**/*.mdx : Provide expected outcomes for each major step in MDX documentation
| ```python | ||
| main = FastMCP("Main") | ||
| mount = main.mount(sub_server, namespace="api") | ||
| mount.add_transform(ToolTransform({...})) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make code blocks runnable (avoid {...} / undefined identifiers) and show an expected outcome.
Right now ToolTransform({...}) and provider = FastMCPProvider(server) (undefined server) aren’t copy/paste runnable. Consider adding a minimal full snippet per section plus a short “Expected: client sees …” verification step. Based on learnings, include expected outcomes.
Also applies to: 324-343
| def check_permissions(ctx: Context) -> str: | ||
| """Check if admin tools should be available.""" | ||
| user = ctx.request_context.get_user() | ||
|
|
||
| if user and user.is_admin: | ||
| mcp.enable(tags={"admin"}) | ||
| return "Admin tools enabled" | ||
| else: | ||
| mcp.disable(tags={"admin"}) | ||
| return "Admin tools disabled" | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -t py "context.py" src/Repository: jlowin/fastmcp
Length of output: 227
🏁 Script executed:
cat -n src/fastmcp/server/context.py | head -100Repository: jlowin/fastmcp
Length of output: 3894
🏁 Script executed:
rg -n "get_user|request_context" --type=py -C3 src/fastmcp/server/context.pyRepository: jlowin/fastmcp
Length of output: 3518
🏁 Script executed:
rg -n "class Context|def get_user" --type=py src/ -A5Repository: jlowin/fastmcp
Length of output: 1344
🏁 Script executed:
rg -n "class RequestContext|def get_user|\.user|user\(" src/fastmcp/ --max-count=20Repository: jlowin/fastmcp
Length of output: 693
🏁 Script executed:
# Check if there are any examples of accessing user or authentication info
rg -n "user|auth|permission" docs/ -i --max-count=15Repository: jlowin/fastmcp
Length of output: 50371
🏁 Script executed:
# Search for actual usage of request_context attributes
rg -n "request_context\." src/fastmcp/ -A2 | head -50Repository: jlowin/fastmcp
Length of output: 2073
🏁 Script executed:
rg -n "get_user|request_context\." docs/servers/visibility.mdx -C5Repository: jlowin/fastmcp
Length of output: 400
🏁 Script executed:
# Check if there are any other examples in the repo of dynamic visibility or accessing user data
rg -n "is_admin|user\." docs/servers/ -C3 | head -40Repository: jlowin/fastmcp
Length of output: 2472
🏁 Script executed:
# Look at the full context section in visibility.mdx around the problematic code
sed -n '260,290p' docs/servers/visibility.mdxRepository: jlowin/fastmcp
Length of output: 918
Replace ctx.request_context.get_user() with a proper user access pattern.
The example uses ctx.request_context.get_user(), which does not exist on the RequestContext API. Instead, access user information through middleware that stores it in context state (using ctx.set_state() and ctx.get_state()), or by examining the access token via get_access_token(). Refer to the Context documentation for the recommended patterns.
Original code that needs correction
@mcp.tool
def check_permissions(ctx: Context) -> str:
"""Check if admin tools should be available."""
user = ctx.request_context.get_user()
if user and user.is_admin:
mcp.enable(tags={"admin"})
return "Admin tools enabled"
else:
mcp.disable(tags={"admin"})
return "Admin tools disabled"| # Re-export built-in transforms (must be after Transform class to avoid circular imports) | ||
| from fastmcp.server.transforms.namespace import Namespace # noqa: E402 | ||
| from fastmcp.server.transforms.tool_transform import ToolTransform # noqa: E402 | ||
| from fastmcp.server.transforms.visibility import Visibility # noqa: E402 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused # noqa: E402 directives (Ruff RUF100).
They’re currently unused per Ruff, so they just add noise.
Proposed diff
-from fastmcp.server.transforms.namespace import Namespace # noqa: E402
-from fastmcp.server.transforms.tool_transform import ToolTransform # noqa: E402
-from fastmcp.server.transforms.visibility import Visibility # noqa: E402
+from fastmcp.server.transforms.namespace import Namespace
+from fastmcp.server.transforms.tool_transform import ToolTransform
+from fastmcp.server.transforms.visibility import Visibility🧰 Tools
🪛 Ruff (0.14.10)
189-189: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
190-190: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
191-191: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e3d97541d
ℹ️ 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".
| if isinstance(result, BaseException): | ||
| logger.debug( | ||
| f"Error during {operation} from provider " | ||
| f"{self._providers[i]}: {result}" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honor mounted_components_raise_on_load_error for list ops
AggregateProvider._collect_list_results always logs provider exceptions at debug level and then drops them, which means server-level list operations (tools/resources/prompts/templates) can no longer surface provider load failures even when fastmcp.settings.mounted_components_raise_on_load_error is True. In that configuration, a mounted provider failing to list (e.g., remote server down) should raise to avoid silently returning partial component lists. Consider reintroducing the flag check (or re-raising) so strict mode continues to fail fast for list operations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/fastmcp/server/server.py (2)
350-352: Redundant import ofToolTransform.
ToolTransformis already imported at line 87. This local import is unnecessary.♻️ Proposed fix
if tool_transformations: if fastmcp.settings.deprecation_warnings: warnings.warn( "The tool_transformations parameter is deprecated. Use " "server.add_transform(ToolTransform({...})) instead.", DeprecationWarning, stacklevel=2, ) - from fastmcp.server.transforms import ToolTransform - self._transforms.append(ToolTransform(dict(tool_transformations)))
987-1000: Unusedtool_nameparameter is intentional for API compatibility.The static analysis correctly identifies that
tool_nameis unused in this no-op deprecated method. Since this method is deprecated and does nothing, keeping the parameter maintains backward compatibility for callers while the deprecation warning guides migration.Consider adding an underscore prefix to silence the linter:
♻️ Proposed fix to silence linter
- def remove_tool_transformation(self, tool_name: str) -> None: + def remove_tool_transformation(self, _tool_name: str) -> None: """Remove a tool transformation. .. deprecated:: Tool transformations are now immutable. Use visibility controls instead. """docs/development/v3-notes/v3-features.mdx (1)
124-140: Custom transform example is missingSequenceimport.The
TagFilterexample usesSequence[Tool]as a return type annotation but doesn't show the import. For completeness and runnable code per MDX guidelines:📝 Proposed fix
-from fastmcp.server.transforms import Transform, ListToolsNext, GetToolNext +from collections.abc import Sequence +from fastmcp.server.transforms import Transform, ListToolsNext, GetToolNext +from fastmcp.tools import Tool class TagFilter(Transform):
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
loq.tomlis excluded by none and included by nonetests/deprecated/test_add_tool_transformation.pyis excluded by none and included by none
📒 Files selected for processing (3)
docs/development/upgrade-guide.mdxdocs/development/v3-notes/v3-features.mdxsrc/fastmcp/server/server.py
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/development/upgrade-guide.mdx
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/development/v3-notes/v3-features.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns
Follow existing patterns and maintain consistency across the codebase
Files:
src/fastmcp/server/server.py
🧬 Code graph analysis (1)
src/fastmcp/server/server.py (4)
src/fastmcp/server/low_level.py (3)
fastmcp(45-50)fastmcp(146-151)get_prompt(248-290)src/fastmcp/server/providers/aggregate.py (11)
AggregateProvider(29-207)list_tools(87-93)get_tool(95-101)list_resources(107-113)get_resource(115-121)list_resource_templates(127-133)get_resource_template(135-141)list_prompts(147-153)get_prompt(155-161)get_tasks(190-195)get_component(167-184)src/fastmcp/server/transforms/namespace.py (9)
Namespace(31-188)list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)src/fastmcp/server/transforms/__init__.py (9)
Transform(46-185)list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py
987-987: Unused method argument: tool_name
(ARG002)
1117-1117: Avoid specifying long messages outside the exception class
(TRY003)
1216-1216: Avoid specifying long messages outside the exception class
(TRY003)
1264-1264: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (7)
src/fastmcp/server/server.py (5)
772-779: LGTM!Creating a new
AggregateProvideron each call is appropriate here since the_providerslist can be modified viaadd_provider(). The approach ensures the aggregate always reflects the current provider state.
789-814: LGTM!The transform chain is correctly built using
partialto create the middleware-stylecall_nextpattern. The chain applies transforms in registration order with visibility as the final filter.
893-946: LGTM!The type-specific transform application is necessary here because each component type has distinct transform methods (
list_tools,list_resources, etc.). This ensures task registration uses correctly transformed names/URIs.
948-966: LGTM!Clean public API with a helpful docstring and example demonstrating the transform's effect across all providers.
2504-2518: LGTM!The transform ordering is correct:
Namespaceis added first so tool names becomenamespace_old_name, thenToolTransformmaps from those namespaced names to the final custom names. The key calculation correctly accounts for whether a namespace is present.docs/development/v3-notes/v3-features.mdx (2)
94-98: LGTM!The example correctly demonstrates the new provider-level transform API with
add_transform(Namespace(...)).
334-345: LGTM!Clear deprecation documentation with a concise migration example showing the transition from
add_tool_transformation()toadd_transform(ToolTransform({...})).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/fastmcp/server/server.py (2)
789-800: Consider caching for performance-sensitive scenarios.
_get_root_provider()creates a newAggregateProviderinstance on every call. While the object is lightweight, in high-throughput scenarios with many component lookups, this could add overhead.This is acceptable for the current design since
AggregateProvideris simple, but if profiling shows this as a hotspot, consider caching the aggregate when providers haven't changed.
953-963: Consider parallelizing transform chain execution.The four transform chains for tools, resources, templates, and prompts are awaited sequentially. Since they're independent, you could use
asyncio.gatherfor a minor performance improvement during startup.♻️ Suggested parallel execution
- transformed_tools = await tools_chain() - transformed_resources = await resources_chain() - transformed_templates = await templates_chain() - transformed_prompts = await prompts_chain() + ( + transformed_tools, + transformed_resources, + transformed_templates, + transformed_prompts, + ) = await asyncio.gather( + tools_chain(), + resources_chain(), + templates_chain(), + prompts_chain(), + )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/development/v3-notes/v3-features.mdxsrc/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns
Follow existing patterns and maintain consistency across the codebase
Files:
src/fastmcp/server/server.py
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/development/v3-notes/v3-features.mdx
🧠 Learnings (1)
📚 Learning: 2026-01-12T16:24:54.978Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2026-01-12T16:24:54.978Z
Learning: Applies to src/tools/**/*.{ts,tsx,js,jsx} : Changes affecting MCP Tools (like adding tags, importing, etc.) must be adopted, applied, and tested consistently
Applied to files:
docs/development/v3-notes/v3-features.mdx
🧬 Code graph analysis (1)
src/fastmcp/server/server.py (13)
src/fastmcp/server/providers/aggregate.py (10)
AggregateProvider(29-207)list_tools(87-93)get_tool(95-101)list_resources(107-113)get_resource(115-121)list_resource_templates(127-133)get_resource_template(135-141)list_prompts(147-153)get_prompt(155-161)get_component(167-184)src/fastmcp/server/tasks/capabilities.py (1)
get_task_capabilities(12-34)src/fastmcp/server/tasks/config.py (2)
TaskConfig(42-152)TaskMeta(26-38)src/fastmcp/server/transforms/namespace.py (9)
Namespace(31-188)list_tools(100-105)get_tool(107-115)list_resources(121-127)get_resource(129-139)list_resource_templates(145-153)get_resource_template(155-167)list_prompts(173-178)get_prompt(180-188)src/fastmcp/server/transforms/tool_transform.py (3)
ToolTransform(15-94)list_tools(63-73)get_tool(75-94)src/fastmcp/server/transforms/__init__.py (9)
Transform(46-185)list_tools(77-86)get_tool(88-98)list_resources(104-113)get_resource(115-127)list_resource_templates(133-144)get_resource_template(146-158)list_prompts(164-173)get_prompt(175-185)src/fastmcp/server/transforms/visibility.py (9)
Visibility(42-304)list_tools(240-243)get_tool(245-250)list_resources(256-259)get_resource(261-268)list_resource_templates(274-279)get_resource_template(281-288)list_prompts(294-297)get_prompt(299-304)src/fastmcp/server/providers/base.py (18)
base(108-109)base(127-128)base(139-140)base(151-152)base(163-164)base(175-176)base(187-188)base(199-200)list_tools(212-217)get_tool(219-229)list_resources(231-236)get_resource(238-248)list_resource_templates(250-255)get_resource_template(257-271)list_prompts(273-278)get_prompt(280-290)add_transform(74-92)Provider(48-440)src/fastmcp/server/providers/proxy.py (4)
list_tools(476-488)list_resources(494-507)list_resource_templates(513-526)list_prompts(532-545)src/fastmcp/server/providers/filesystem.py (8)
list_tools(176-179)get_tool(181-184)list_resources(186-189)get_resource(191-194)list_resource_templates(196-199)get_resource_template(201-204)list_prompts(206-209)get_prompt(211-214)src/fastmcp/resources/resource.py (3)
Resource(214-409)key(380-382)resource(510-640)src/fastmcp/resources/template.py (2)
ResourceTemplate(101-316)key(285-287)src/fastmcp/server/auth/authorization.py (3)
tool(63-70)AuthContext(47-70)run_auth_checks(147-190)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py
1146-1146: Avoid specifying long messages outside the exception class
(TRY003)
1153-1153: Avoid specifying long messages outside the exception class
(TRY003)
1216-1216: Avoid specifying long messages outside the exception class
(TRY003)
1223-1223: Avoid specifying long messages outside the exception class
(TRY003)
1288-1288: Avoid specifying long messages outside the exception class
(TRY003)
1295-1295: Avoid specifying long messages outside the exception class
(TRY003)
1358-1358: Avoid specifying long messages outside the exception class
(TRY003)
1365-1365: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (17)
docs/development/v3-notes/v3-features.mdx (3)
27-27: LGTM!The transform stacking bullet point clearly introduces the capability with appropriate method examples.
152-152: LGTM!The file path reference correctly points to the new transforms module location.
387-398: LGTM with minor suggestion.The deprecation migration path is clear. For enhanced clarity, consider showing what
configrepresents (e.g.,ToolTransformConfig(name="new_name")) to make the example more concrete, similar to the pattern shown in lines 116-118.src/fastmcp/server/server.py (14)
84-92: LGTM!The new imports for the transform system are cleanly organized and follow the existing import patterns in the file.
283-290: LGTM!Server-level transforms list is properly initialized, and the local provider is correctly positioned as first in the provider chain.
806-817: LGTM!The middleware chain pattern is correctly implemented. Transforms execute in order with visibility applied last (filtering the final aggregated results), which aligns with the PR objectives.
819-830: LGTM!The get-by-name variant correctly passes the name through the transform chain, allowing transforms like
Namespaceto modify the lookup name.
965-983: LGTM!Clean public API for adding server-level transforms with good documentation. The example in the docstring is helpful.
Note: Adding transforms after the server has started could lead to inconsistent behavior for in-flight requests, but this appears to be an acceptable design choice matching the existing provider modification patterns.
985-1015: LGTM!Deprecation paths are well-implemented with clear warnings and migration guidance. The
remove_tool_transformationcorrectly indicates it has no effect and suggests using visibility controls instead.
1115-1136: LGTM!The refactored
get_toolscorrectly applies the full transform chain before deduplication and authorization. The exception handling forAuthorizationErrorduring list operations (treating as denial rather than raising) is a good pattern.
1138-1155: LGTM!The refactored
get_toolproperly uses the transform chain. ReturningNotFoundErrorfor both missing and unauthorized tools is a security-conscious choice that prevents leaking component existence.
1369-1394: LGTM!The
get_componentrouting correctly delegates to transform-enabledget_*methods, ensuring consistent behavior regardless of how components are accessed.
516-526: LGTM!Good defensive error handling for task component discovery. The fallback to an empty list with logging ensures server startup isn't blocked by non-critical transform errors, while still respecting the
mounted_components_raise_on_load_errorsetting.
2616-2630: LGTM!The mount implementation correctly applies transforms to the provider. The key insight is that
tool_namesmust reference namespaced names (e.g.,"weather_get_forecast"→"forecast") because theNamespacetransform is added first and processes beforeToolTransform.
2765-2776: LGTM!Factory methods are cleanly updated. The type annotation broadening to
Providermaintains flexibility while the simplifiedcreate_proxyreturn aligns with the new transform-based architecture.Also applies to: 2822-2832, 2935-2938
259-259: LGTM!Backward compatibility for the deprecated
tool_transformationsparameter is handled correctly with a clear deprecation warning and automatic migration to the newadd_transform(ToolTransform(...))approach.Also applies to: 360-369
1-100: Overall assessment: Well-designed transform system.This PR introduces a clean middleware-style transform system that integrates well with the existing FastMCP architecture. Key strengths:
- Consistent patterns: The
_source_*methods follow a uniform chain-building pattern- Correct ordering: Visibility is correctly applied last in the transform chain
- Good deprecation paths: Legacy APIs (
add_tool_transformation,tool_transformationsparam) are preserved with clear migration guidance- Defensive error handling: Task discovery gracefully handles errors without blocking startup
The implementation aligns well with the PR objectives of providing composable, middleware-style transforms for component modification.
Test Failure AnalysisSummary: The static analysis check failed because has grown to 539 lines, exceeding the configured limit of 536 lines in Root Cause: The PR adds documentation content to [[rules]]
path = "docs/development/v3-notes/v3-features.mdx"
max_lines = 536The file now exceeds this limit by 3 lines. Suggested Solution: Update the [[rules]]
path = "docs/development/v3-notes/v3-features.mdx"
max_lines = 539Alternatively, merge with main first - the main branch has updated Detailed AnalysisLog excerpt showing the failure: Changes made in PR:
Main branch difference:
Related Files
|
Test Failure AnalysisSummary: Windows tests timing out during SQLite connection in OAuthProxy fixture initialization. Root Cause: The Suggested Solution: Add # Line 291 fixture
@pytest.fixture
def oauth_proxy(self):
from key_value.aio.stores.memory import MemoryStore
return OAuthProxy(
# ... existing params ...
client_storage=MemoryStore(), # Add this
jwt_signing_key="test-secret",
)This matches the pattern already used in Detailed AnalysisFailing test: Stack trace shows: The test hung for the full timeout period trying to establish an SQLite connection on Windows. Why this happens: Windows has different filesystem locking behavior and SQLite connection handling compared to Unix systems. The DiskStore initialization is trying to create/connect to a SQLite database file, which is encountering issues specific to the Windows runner environment. Why MemoryStore fixes it: Using MemoryStore avoids all filesystem operations and provides the same AsyncKeyValue interface that OAuthProxy needs for client storage, without any disk or SQLite dependencies. Related Files
|
Test Failure AnalysisSummary: The pre-commit hook failed because exceeded its file size limit (539 lines > 536 lines allowed). Root Cause: The PR added 3 lines to , pushing it from 536 lines to 539 lines. While has a general exclude for (line 10), there's a specific rule for this file (line 310-311) that sets a limit of 536 lines. Specific rules override the general exclude. Suggested Solution: Update the file to increase the line limit for this file. Change line 311 from: [[rules]]
path = "docs/development/v3-notes/v3-features.mdx"
max_lines = 536to: [[rules]]
path = "docs/development/v3-notes/v3-features.mdx"
max_lines = 539Or, according to the development guidelines, run uv run loq baselineDetailed AnalysisThe failure occurred in the The file currently has 539 lines (confirmed by Related Files
|
Transforms modify components (tools, resources, prompts) as they flow from providers to clients. They use a middleware pattern where each transform receives a
call_nextcallable to continue the chain.Custom transforms subclass
Transformand override the methods they need:Visibility (enable/disable filtering) is implemented as a transform, providing a unified model - provider-level visibility sees original names, server-level visibility sees transformed names.
Documentation at
docs/servers/providers/transforms.mdxanddocs/servers/visibility.mdx.