Refactor FastMCPProxy into ProxyProvider#2669
Conversation
Move proxy functionality from custom manager classes to the Provider pattern: - Create ProxyProvider that implements the Provider interface - Move all proxy code to src/fastmcp/server/providers/proxy.py - Keep FastMCPProxy as a convenience wrapper using ProxyProvider - Add deprecation warning when importing from old location - Convert handler classmethods to module-level functions - Remove redundant get_* methods (base class defaults work)
marvin-context-protocol
Bot
added
enhancement
WalkthroughMoves proxy implementations into a new module Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/fastmcp/server/proxy.py (1)
19-29: Remove unusednoqadirective.The
# noqa: E402comment is unnecessary here. The deprecation warning on lines 12-16 doesn't trigger the E402 rule (module level import not at top of file) because there are no preceding executable statements that would cause this issue —warnings.warn()is called before the imports, which is intentional and valid.🔎 Proposed fix
-from fastmcp.server.providers.proxy import ( # noqa: E402 +from fastmcp.server.providers.proxy import ( ClientFactoryT,
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
tests/server/proxy/test_proxy_client.pyis excluded by none and included by nonetests/server/proxy/test_proxy_server.pyis excluded by none and included by nonetests/server/proxy/test_stateful_proxy_client.pyis excluded by none and included by nonetests/server/tasks/test_task_proxy.pyis excluded by none and included by nonetests/server/test_mount.pyis excluded by none and included by none
📒 Files selected for processing (6)
src/fastmcp/client/transports.pysrc/fastmcp/server/providers/__init__.pysrc/fastmcp/server/providers/proxy.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/server.pysrc/fastmcp/utilities/mcp_config.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/utilities/mcp_config.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/providers/__init__.pysrc/fastmcp/server/server.pysrc/fastmcp/client/transports.pysrc/fastmcp/server/providers/proxy.py
src/fastmcp/**/__init__.py
📄 CodeRabbit inference engine (AGENTS.md)
Core types that define a module's purpose should be exported (e.g.,
Middlewarefromfastmcp.server.middleware), while specialized features can live in submodules
Files:
src/fastmcp/server/providers/__init__.py
🧠 Learnings (3)
📚 Learning: 2025-12-21T21:37:55.015Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to src/fastmcp/__init__.py : All module exports should be intentional - only re-export to `fastmcp.*` for fundamental types like `FastMCP` and `Client`, prefer users importing from specific submodules for specialized features
Applied to files:
src/fastmcp/utilities/mcp_config.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/providers/__init__.pysrc/fastmcp/server/server.pysrc/fastmcp/server/providers/proxy.py
📚 Learning: 2025-12-21T21:37:55.015Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to src/fastmcp/**/__init__.py : Core types that define a module's purpose should be exported (e.g., `Middleware` from `fastmcp.server.middleware`), while specialized features can live in submodules
Applied to files:
src/fastmcp/utilities/mcp_config.pysrc/fastmcp/server/proxy.pysrc/fastmcp/server/providers/__init__.pysrc/fastmcp/server/server.py
📚 Learning: 2025-12-21T21:37:55.015Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to tests/**/*.py : Use in-memory transport (pass FastMCP servers directly to clients) for testing, only use HTTP transport when explicitly testing network features
Applied to files:
src/fastmcp/client/transports.py
🧬 Code graph analysis (5)
src/fastmcp/utilities/mcp_config.py (1)
src/fastmcp/server/providers/proxy.py (2)
FastMCPProxy(562-604)ProxyClient(683-717)
src/fastmcp/server/providers/__init__.py (1)
src/fastmcp/server/providers/proxy.py (1)
ProxyProvider(408-554)
src/fastmcp/server/server.py (1)
src/fastmcp/server/providers/proxy.py (2)
FastMCPProxy(562-604)ProxyClient(683-717)
src/fastmcp/client/transports.py (1)
src/fastmcp/server/server.py (1)
name(355-356)
src/fastmcp/server/providers/proxy.py (11)
src/fastmcp/client/client.py (1)
Client(127-1627)src/fastmcp/mcp_config.py (1)
MCPConfig(243-294)src/fastmcp/prompts/prompt.py (2)
Prompt(118-322)PromptResult(73-115)src/fastmcp/resources/resource.py (1)
Resource(137-331)src/fastmcp/resources/template.py (1)
ResourceTemplate(97-298)src/fastmcp/server/context.py (3)
Context(117-1003)request_context(212-238)log(309-335)src/fastmcp/server/dependencies.py (1)
get_context(271-277)src/fastmcp/server/providers/base.py (1)
Provider(66-384)src/fastmcp/tools/tool.py (1)
Tool(126-369)src/fastmcp/tools/tool_transform.py (2)
ToolTransformConfig(878-921)apply_transformations_to_tools(924-943)src/fastmcp/utilities/components.py (1)
MirroredComponent(156-192)
🪛 Ruff (0.14.8)
src/fastmcp/server/proxy.py
19-19: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
src/fastmcp/server/providers/__init__.py
51-51: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/providers/proxy.py
220-222: Avoid specifying long messages outside the exception class
(TRY003)
236-236: Avoid specifying long messages outside the exception class
(TRY003)
288-288: Unused method argument: uri
(ARG002)
290-290: Unused method argument: context
(ARG002)
305-307: Avoid specifying long messages outside the exception class
(TRY003)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
613-613: Unused function argument: context
(ARG001)
623-623: Unused function argument: context
(ARG001)
645-645: Unused function argument: response_type
(ARG001)
647-647: Unused function argument: context
(ARG001)
⏰ 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 (19)
src/fastmcp/client/transports.py (1)
1018-1020: LGTM!The parameter rename from
prefixtonamespacecorrectly aligns with the updatedmount()API signature inFastMCP.server. The conditional logic based onname_as_prefixis preserved.src/fastmcp/utilities/mcp_config.py (1)
13-13: LGTM!Import path correctly updated to the new canonical location
fastmcp.server.providers.proxy. This aligns with the module reorganization and avoids triggering the deprecation warning from the old module.src/fastmcp/server/server.py (3)
102-102: LGTM!TYPE_CHECKING import correctly updated to reference
FastMCPProxyfrom its new canonical location.
2634-2634: LGTM!Runtime import in
mount()correctly updated for backward compatibility handling of the deprecatedas_proxyparameter.
2839-2839: LGTM!Runtime import in
as_proxy()correctly updated to import bothFastMCPProxyandProxyClientfrom the new module location.src/fastmcp/server/proxy.py (2)
1-16: LGTM!The compatibility shim is well-structured with a clear docstring explaining the deprecation and proper use of
stacklevel=2for the warning to point to the caller's import statement.
31-41: LGTM!The
__all__declaration correctly lists all the re-exported symbols, maintaining the same public API surface as the new module location.src/fastmcp/server/providers/__init__.py (2)
28-42: LGTM!The TYPE_CHECKING import combined with
__all__export correctly enables static type checkers to recognizeProxyProvideras a public symbol without triggering a runtime import.
45-51: LGTM!The lazy import pattern via
__getattr__is correctly implemented to avoid circular imports while makingProxyProvideraccessible asfastmcp.server.providers.ProxyProvider. This is a standard Python pattern for deferred module loading.src/fastmcp/server/providers/proxy.py (10)
1-56: LGTM!Module setup is clean with appropriate imports and a clear type alias for
ClientFactoryT. The organization separates concerns well between component proxies, the provider, and client utilities.
67-111: LGTM!
ProxyToolcorrectly implements the proxy pattern:
task_config.mode="forbidden"prevents task execution through proxies_backend_namepreserves the original name when namespace transforms are applied viamodel_copyfrom_mcp_toolfactory properly maps MCP schema including extracting tags from meta
112-152: LGTM!The
run()method correctly forwards tool execution to the remote server, including proper handling of task metadata when present in the request context. Error handling converts remote errors toToolErrorappropriately.
154-237: LGTM!
ProxyResourcefollows the same well-designed pattern asProxyTool. Theread()method correctly handles both text and blob content types from the remote server.
239-335: LGTM!
ProxyTemplatecorrectly handles parameterized URIs. Thecreate_resourcemethod intentionally ignores the provideduriparameter (as documented in the comment) because the URI may differ after namespace transforms — it reconstructs the backend URI from the original template and provided params.
337-401: LGTM!
ProxyPromptcorrectly forwards prompt rendering to the remote server and preserves runtime meta from the result.
408-554: LGTM!
ProxyProvidercleanly implements theProviderinterface:
list_*methods gracefully handleMETHOD_NOT_FOUNDfor servers that don't support all component typesget_tasks()returns empty components since proxied components can't execute as background tasks- Tool transformations are applied when configured
562-604: LGTM!
FastMCPProxyis a clean convenience wrapper that creates aFastMCPserver with aProxyProvider. Thetool_transformationsextraction before callingsuper().__init__prevents the parameter from being passed toFastMCPwhich doesn't accept it.
612-681: LGTM!The default proxy handlers correctly forward requests from the remote server to the proxy's connected clients. The unused
contextparameters are intentional — they're part of the handler callback signatures but the actual context is obtained viaget_context()which accesses the current server's context.
683-766: LGTM!
ProxyClientandStatefulProxyClientare well-designed:
ProxyClientsets up sensible defaults for all proxy handler callbacksStatefulProxyClientcorrectly manages per-session client caching with proper cleanup viasession._exit_stack
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/fastmcp/server/providers/proxy.py (1)
308-321: Consider extracting duplicate content conversion logic.The content type handling logic (lines 308-321) is duplicated from
ProxyResource.read()(lines 223-236). Consider extracting this into a shared helper method to reduce duplication and improve maintainability.🔎 Suggested refactor
def _convert_mcp_resource_content( content: TextResourceContents | BlobResourceContents, ) -> ResourceContent: """Convert MCP resource content to ResourceContent.""" if isinstance(content, TextResourceContents): return ResourceContent( content=content.text, mime_type=content.mimeType, meta=content.meta, ) elif isinstance(content, BlobResourceContents): return ResourceContent( content=base64.b64decode(content.blob), mime_type=content.mimeType, meta=content.meta, ) else: raise ResourceError(f"Unsupported content type: {type(content)}")Then use it in both
ProxyResource.read()andProxyTemplate.create_resource().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/fastmcp/server/providers/base.pysrc/fastmcp/server/providers/proxy.py
💤 Files with no reviewable changes (1)
- src/fastmcp/server/providers/base.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/providers/proxy.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to src/fastmcp/__init__.py : All module exports should be intentional - only re-export to `fastmcp.*` for fundamental types like `FastMCP` and `Client`, prefer users importing from specific submodules for specialized features
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to src/fastmcp/**/__init__.py : Core types that define a module's purpose should be exported (e.g., `Middleware` from `fastmcp.server.middleware`), while specialized features can live in submodules
🪛 Ruff (0.14.8)
src/fastmcp/server/providers/proxy.py
220-222: Avoid specifying long messages outside the exception class
(TRY003)
236-236: Avoid specifying long messages outside the exception class
(TRY003)
288-288: Unused method argument: uri
(ARG002)
290-290: Unused method argument: context
(ARG002)
305-307: Avoid specifying long messages outside the exception class
(TRY003)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
613-613: Unused function argument: context
(ARG001)
623-623: Unused function argument: context
(ARG001)
645-645: Unused function argument: response_type
(ARG001)
647-647: Unused function argument: context
(ARG001)
⏰ 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). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (2)
src/fastmcp/server/providers/proxy.py (2)
764-764: Note: This code relies on private MCP SDK internals that could break with SDK updates.Line 764 accesses
session._exit_stack, a private attribute of ServerSession. The MCP SDK does not expose a public API for registering session exit callbacks. This creates a fragile dependency on internal implementation details. Consider pinning the MCP SDK version and adding a comment documenting this reliance, or investigate whether FastMCP's session event system (connect/disconnect events) provides an alternative approach.
462-538: Consider implementing parallel loading for provider initialization.The
list_tools(),list_resources(),list_resource_templates(), andlist_prompts()methods currently load from a single client sequentially. If these are called together during initialization, usingasyncio.gather()to load them in parallel could improve startup time.Note: This should be verified against your PR scope and requirements. Parallel loading may be intentionally deferred or implemented at a different layer (e.g., when multiple providers are mounted to a FastMCP instance).
| async def run( | ||
| self, | ||
| arguments: dict[str, Any], | ||
| context: Context | None = None, | ||
| ) -> ToolResult: | ||
| """Executes the tool by making a call through the client.""" | ||
| client = await self._get_client() | ||
| async with client: | ||
| context = get_context() |
There was a problem hiding this comment.
Unused context parameter is immediately overwritten.
The context parameter in the run() method signature is never used—it's immediately overwritten by get_context() at Line 120. This is misleading for callers who might pass a context expecting it to be used.
🔎 Proposed fix
Either remove the parameter if it's not required by the parent signature, or use the provided context when available:
async def run(
self,
arguments: dict[str, Any],
- context: Context | None = None,
) -> ToolResult:
"""Executes the tool by making a call through the client."""
client = await self._get_client()
async with client:
context = get_context()Or if you want to support both patterns:
async def run(
self,
arguments: dict[str, Any],
context: Context | None = None,
) -> ToolResult:
"""Executes the tool by making a call through the client."""
client = await self._get_client()
async with client:
- context = get_context()
+ context = context or get_context()📝 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.
| async def run( | |
| self, | |
| arguments: dict[str, Any], | |
| context: Context | None = None, | |
| ) -> ToolResult: | |
| """Executes the tool by making a call through the client.""" | |
| client = await self._get_client() | |
| async with client: | |
| context = get_context() | |
| async def run( | |
| self, | |
| arguments: dict[str, Any], | |
| context: Context | None = None, | |
| ) -> ToolResult: | |
| """Executes the tool by making a call through the client.""" | |
| client = await self._get_client() | |
| async with client: | |
| context = context or get_context() |
🤖 Prompt for AI Agents
In src/fastmcp/server/providers/proxy.py around lines 112 to 120, the run()
method's context parameter is immediately overwritten by get_context(), making
the passed-in context unused; either remove the parameter if it's not required
by the parent signature, or preserve and use the provided context by replacing
the overwrite with logic like "if context is None: context = get_context()" (or
equivalent) so the caller-supplied Context is honored while still falling back
to get_context() when none is provided.
| if result.isError: | ||
| raise ToolError(cast(mcp.types.TextContent, result.content[0]).text) |
There was a problem hiding this comment.
Validate content type before casting to TextContent.
When result.isError is true, the code assumes content[0] is a TextContent and casts it without validation. If the remote server returns a different content type in the error response, this will fail at runtime when accessing .text.
🔎 Proposed fix
if result.isError:
- raise ToolError(cast(mcp.types.TextContent, result.content[0]).text)
+ error_content = result.content[0] if result.content else None
+ if isinstance(error_content, mcp.types.TextContent):
+ raise ToolError(error_content.text)
+ else:
+ raise ToolError("Tool execution failed with non-text error content")🤖 Prompt for AI Agents
In src/fastmcp/server/providers/proxy.py around lines 144-145, the code
unconditionally casts result.content[0] to mcp.types.TextContent and accesses
.text when result.isError is true; validate the content type before casting
(e.g., check isinstance(content[0], mcp.types.TextContent) or check a
type/member indicator) and only access .text when safe, otherwise fall back to a
safe string representation (like str(content[0]) or content[0].get("message") if
it's a dict) when constructing the ToolError; ensure you handle empty content
lists and include a clear fallback error message so the raise never assumes a
TextContent shape.
jlowin
removed
the
breaking change
1 participant
Refactors proxy functionality from custom manager classes (
ProxyToolManager,ProxyResourceManager, etc.) to use the Provider pattern.Changes
ProxyProvidernow implements theProviderinterface, returningProxyTool,ProxyResource,ProxyTemplate, andProxyPromptcomponents that forward execution to remote servers.FastMCPProxyremains as a convenience wrapper that internally usesProxyProvider:All proxy code moves to
fastmcp.server.providers.proxy. The oldfastmcp.server.proxylocation emits a deprecation warning.