Unify discovery API: deduplicate at protocol layer only#2919
Conversation
…otocol layer list_tools() now returns all versions with a run_middleware flag to avoid recursion. Deduplication moves to protocol handlers only, enabling version requests to flow through mounted server hierarchies.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR replaces public get_* APIs with list_* equivalents (get_tools/get_resources/get_prompts/get_resource_templates → list_tools/list_resources/list_prompts/list_resource_templates), flips the default run_middleware to True and returns Sequence types, adds deduplication in MCP list handlers, and threads explicit VersionSpec/version through FastMCPProvider and server call paths (run/read/render/call). It also updates provider internals to call non-middleware list methods, adjusts utilities and example code to use the new list_* names, and adds a small signature/annotation change in an example function. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Actionable comments posted: 1
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/providers/fastmcp_provider.py (1)
117-138: Forward versions for resources/prompts/templates too.Tools now pass exact versions to the child server, but resource/prompt/template wrappers still call without version, so a version-specific request can resolve the wrong version in mounted hierarchies. Please propagate the wrapper’s version in those delegate calls as well.
Based on learnings, maintain consistency across Tools, Resources, Resource Templates, and Prompts.✅ Suggested fix to keep all component types consistent
class FastMCPProviderResource(Resource): @@ async def _read( self, task_meta: TaskMeta | None = None ) -> ResourceResult | mcp.types.CreateTaskResult: @@ - with delegate_span( + version = VersionSpec(eq=self.version) if self.version else None + with delegate_span( self._original_uri or "", "FastMCPProvider", self._original_uri or "" ): return await self._server.read_resource( - self._original_uri, task_meta=task_meta + self._original_uri, version=version, task_meta=task_meta ) class FastMCPProviderPrompt(Prompt): @@ async def _render( self, arguments: dict[str, Any] | None = None, task_meta: TaskMeta | None = None, ) -> PromptResult | mcp.types.CreateTaskResult: @@ - with delegate_span( + version = VersionSpec(eq=self.version) if self.version else None + with delegate_span( self._original_name or "", "FastMCPProvider", self._original_name or "" ): return await self._server.render_prompt( - self._original_name, arguments, task_meta=task_meta + self._original_name, arguments, version=version, task_meta=task_meta ) async def render(self, arguments: dict[str, Any] | None = None) -> PromptResult: @@ - result = await self._server.render_prompt(self._original_name, arguments) + version = VersionSpec(eq=self.version) if self.version else None + result = await self._server.render_prompt( + self._original_name, arguments, version=version + ) class FastMCPProviderResourceTemplate(ResourceTemplate): @@ async def _read( self, uri: str, params: dict[str, Any], task_meta: TaskMeta | None = None ) -> ResourceResult | mcp.types.CreateTaskResult: @@ - with delegate_span( + version = VersionSpec(eq=self.version) if self.version else None + with delegate_span( original_uri, "FastMCPProvider", self._original_uri_template or "" ): - return await self._server.read_resource(original_uri, task_meta=task_meta) + return await self._server.read_resource( + original_uri, version=version, task_meta=task_meta + ) async def read(self, arguments: dict[str, Any]) -> str | bytes | ResourceResult: @@ - result = await self._server.read_resource(original_uri) + version = VersionSpec(eq=self.version) if self.version else None + result = await self._server.read_resource(original_uri, version=version)
🧹 Nitpick comments (1)
src/fastmcp/server/server.py (1)
1008-1041: Silence unused middleware lambda arg to satisfy Ruff (ARG005).Rename the unused lambda parameter to
_contextto keep lint clean without behavior changes.♻️ Proposed cleanup
- call_next=lambda context: self.list_tools(run_middleware=False), + call_next=lambda _context: self.list_tools(run_middleware=False), @@ - call_next=lambda context: self.list_resources(run_middleware=False), + call_next=lambda _context: self.list_resources(run_middleware=False), @@ - call_next=lambda context: self.list_resource_templates( + call_next=lambda _context: self.list_resource_templates( run_middleware=False ), @@ - call_next=lambda context: self.list_prompts(run_middleware=False), + call_next=lambda _context: self.list_prompts(run_middleware=False),Also applies to: 1095-1130, 1183-1222, 1275-1308
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c307eb4f76
ℹ️ 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".
Phase B of the provider refactor (follows #2913): consolidates the discovery API surface.
Before: Four methods per component type -
list_tools()(raw),get_tools()(filtered + deduplicated),_list_tools()(override point),_get_tool()(override point).After:
list_tools()returns all versions with server-specific filtering (enabled, auth, middleware). Deduplication happens only in protocol handlers when serializing for MCP clients.This enables version requests to flow through mounted server hierarchies - a parent can now request any version from a child, not just the deduplicated "latest" version.
The
run_middleware: bool = Trueparameter prevents recursion when middleware wraps list calls.