-
Notifications
You must be signed in to change notification settings - Fork 4
task: Changes required for FastMCP v3.x #425
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -54,7 +54,7 @@ def mcp_create_server(server_name: str = MCP_SERVER_NAME) -> FastMCP: | |||||||||
|
|
||||||||||
| Creates a new FastMCP server instance and mounts all discovered MCP servers | ||||||||||
| from the SDK and plugins. Each mounted server's tools are namespaced | ||||||||||
| automatically using FastMCP's built-in prefix feature. | ||||||||||
| automatically using FastMCP's built-in namespace feature. | ||||||||||
|
|
||||||||||
| Args: | ||||||||||
| server_name: Human-readable name for the MCP server. | ||||||||||
|
|
@@ -76,7 +76,7 @@ def mcp_create_server(server_name: str = MCP_SERVER_NAME) -> FastMCP: | |||||||||
| continue | ||||||||||
| seen_names.add(server.name) | ||||||||||
| logger.info(f"Mounting MCP server: {server.name}") | ||||||||||
| mcp.mount(server, prefix=server.name) | ||||||||||
| mcp.mount(server, namespace=server.name) | ||||||||||
| count += 1 | ||||||||||
|
|
||||||||||
| logger.info(f"Mounted {count} MCP servers") | ||||||||||
|
|
@@ -115,7 +115,7 @@ def mcp_list_tools(server_name: str = MCP_SERVER_NAME) -> list[dict[str, Any]]: | |||||||||
| 'name' and 'description' keys. | ||||||||||
| """ | ||||||||||
| server = mcp_create_server(server_name) | ||||||||||
| # FastMCP's get_tools() is async because mounted servers may need to | ||||||||||
| # FastMCP's list_tools() is async because mounted servers may need to | ||||||||||
| # lazily initialize resources. We use asyncio.run() to bridge sync/async. | ||||||||||
| tools = asyncio.run(server.get_tools()) | ||||||||||
| return [{"name": name, "description": tool.description or ""} for name, tool in tools.items()] | ||||||||||
| tools = asyncio.run(server.list_tools()) | ||||||||||
|
||||||||||
| return [{"name": tool.name, "description": tool.description or ""} for tool in tools] | ||||||||||
|
||||||||||
| return [{"name": tool.name, "description": tool.description or ""} for tool in tools] | |
| # Sort tools by name to ensure deterministic output order across environments | |
| sorted_tools = sorted(tools, key=lambda tool: tool.name) | |
| return [{"name": tool.name, "description": tool.description or ""} for tool in sorted_tools] |
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.
Bug: Tests in mcp_test.py use the outdated FastMCP v2 API (get_tools()), while production code uses the new v3 API (list_tools()), guaranteeing future test failures.
Severity: CRITICAL
Suggested Fix
Update the test file tests/aignostics/utils/mcp_test.py to use the new FastMCP v3.x API. Replace calls to server.get_tools() with server.list_tools(). Update the test logic to handle a list of Tool objects instead of a dictionary, accessing tool names via the tool.name attribute.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/aignostics/utils/_mcp.py#L120-L121
Potential issue: The production code was updated to use the FastMCP v3.x API,
specifically changing from `server.get_tools()` to `server.list_tools()`. However, the
corresponding test file, `tests/aignostics/utils/mcp_test.py`, was not updated. It still
calls the old `get_tools()` method and expects a dictionary, while the new
`list_tools()` method returns a list of `Tool` objects. When the FastMCP dependency is
updated to v3.x as intended for this pull request, the tests will fail with an
`AttributeError`, blocking deployment.
Did we get this right? 👍 / 👎 to inform future reviews.
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.
namespaceonmount()andlist_tools()are FastMCP v3.x API calls; if this PR lands before the dependency is bumped to v3.x, this will raise at runtime (unexpected keyword argument / missing attribute). To make this change safe (and keep CI green) untilpyproject.tomlis updated, consider supporting both APIs via capability detection (e.g., trynamespacethen fallback toprefix, and trylist_tools()then fallback toget_tools()).