Making MCP Server Aggregator Cloud ready #473
Conversation
WalkthroughExposes two example routines as public app tools returning strings in the MCP Server Aggregator example, adjusting result accumulation and logging. Updates the README with a new “[Beta] Deploy to the cloud” section detailing login, deploy, and client connection steps with config examples. No other modules changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as MCP Client
participant App as Example App (@app.tool)
participant Agg as MCPAggregator
participant Servers as MCP Servers
rect rgba(220,240,255,0.4)
Note over Client,App: Tool invocation (persistent or non-persistent)
Client->>App: Call example_usage(_persistent)
App->>Agg: init(connection_persistence=bool)
end
rect rgba(235,255,235,0.4)
App->>Agg: list_tools()
Agg->>Servers: Query available tools
Servers-->>Agg: Tools list
Agg-->>App: Tools list
App->>App: Append to result
end
rect rgba(235,255,235,0.4)
App->>Agg: read_text_file(path="README.md")
Agg->>Servers: read_text_file
Servers-->>Agg: File contents
Agg-->>App: Output
App->>App: Append to result
end
rect rgba(235,255,235,0.4)
App->>Agg: fetch_fetch(...)
Agg->>Servers: fetch
Servers-->>Agg: Response
Agg-->>App: Output
App->>App: Append to result
end
App-->>Client: Return accumulated string result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
non-persistent mode fails for the filesystem mcp server: non-blocking issue, merging for now |
![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 (11)
examples/basic/mcp_server_aggregator/main.py (11)
20-20: Initialize result with proper spacing.Consider adding a space after the equals sign for better readability.
- result="" + result = ""
39-39: Add spacing in string concatenation for better formatting.The concatenation lacks spacing which could make the output harder to read.
- result+="Tools available:"+str(output) + result += "Tools available: " + str(output)
47-47: Improve string concatenation formatting.Add proper spacing for better readability.
- result+="\n\nread_text_file result:" + str(output) + result += "\n\nread_text_file result: " + str(output)
56-56: Improve string concatenation formatting.Add proper spacing for consistency.
- result+=f"\n\nfetch result: {str(output)}" + result += f"\n\nfetch result: {str(output)}"
61-61: Add null check before closing aggregator.The aggregator could be None if creation fails before the assignment completes.
- await aggregator.close() + if aggregator: + await aggregator.close()
73-73: Initialize result with proper spacing.- result="" + result = ""
92-92: Add spacing in string concatenation.- result+="Tools available:"+str(output) + result += "Tools available: " + str(output)
100-100: Improve string concatenation formatting.- result+="\n\nread_text_file result:" + str(output) + result += "\n\nread_text_file result: " + str(output)
108-109: Inconsistent logging approach and formatting.Line 108 uses f-string in the message while other locations use the
dataparameter. Also missing proper spacing in concatenation on line 109.- logger.info(f"fetch result: {str(output)}") - result+=f"\n\nfetch result: {str(output)}" + logger.info("fetch result:", data=output) + result += f"\n\nfetch result: {str(output)}"
114-114: Add null check before closing aggregator.- await aggregator.close() + if aggregator: + await aggregator.close()
116-116: Consider removing print statement in tool function.Since this function is now a tool that returns a string result, the print statement may be redundant - the caller/framework will handle output display.
Consider removing the print statement as the return value should be sufficient for cloud deployment scenarios where the output is handled by the MCP framework.
- print(result) - return result
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/basic/mcp_server_aggregator/README.md(1 hunks)examples/basic/mcp_server_aggregator/main.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/basic/mcp_server_aggregator/main.py (3)
src/mcp_agent/server/app_server.py (1)
app(134-136)src/mcp_agent/app.py (1)
logger(193-210)src/mcp_agent/logging/logger.py (2)
info(271-279)error(291-299)
🔇 Additional comments (4)
examples/basic/mcp_server_aggregator/README.md (1)
61-114: Cloud deployment documentation looks comprehensive and well-structured.The new cloud deployment section provides clear step-by-step instructions for deploying the MCP Server Aggregator. The documentation includes essential configuration examples for both Claude Desktop and MCP Inspector integrations, with appropriate emphasis on the timeout settings for LLM calls.
examples/basic/mcp_server_aggregator/main.py (3)
11-19: Good transformation to cloud-compatible tool interface.Converting
example_usage_persistentto an@app.tooldecorated function with a string return type makes it accessible as an MCP tool in the cloud deployment. The docstring clearly explains the persistent connection behavior.
43-43: Tool name change aligns with filesystem server's actual interface.The change from
read_filetoread_text_filecorrectly matches the actual tool name exposed by the@modelcontextprotocol/server-filesystempackage.
65-72: Non-persistent mode implementation with known filesystem server limitation.As noted in the PR objectives, the non-persistent mode fails with the filesystem server because it requires stateful roots negotiation. This is a known issue being tracked separately. The tool decorator and return type changes are appropriate for cloud deployment.
Since this is a known issue with the filesystem server in non-persistent mode, consider adding a comment in the code to document this limitation until it's resolved.
@app.tool async def example_usage()->str: ''' this example function/tool call will use an MCP aggregator to connect to both the file and filesystem servers and aggregate them together, so you can list all tool calls from - both servers at once. + both servers at once. + + Note: The filesystem server may fail in non-persistent mode due to + stateful roots negotiation requirements. '''
2 participants
Local run:
Cloud run:
To do:
Summary by CodeRabbit
New Features
Documentation