Feat/fast mcp settings #361
Conversation
|
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. WalkthroughThe changes introduce support for custom FastMCP server settings in the MCP agent server asyncio example. Command-line flags and argument parsing are added to both the server and client scripts, allowing users to optionally enable custom settings. The server creation utility is updated to accept and apply these settings, and documentation is revised accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Server
participant AppServerUtility
User->>Client: Run client.py [--custom-fastmcp-settings]
Client->>Server: Start basic_agent_server.py [with flag if set]
Server->>AppServerUtility: create_mcp_server_for_app(app, settings)
AppServerUtility->>Server: Returns FastMCP server instance (custom/default settings)
Server->>User: Server starts with chosen settings
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/mcp_agent/server/app_server.py (1)
125-125: Safe parameter unpacking implementation.The use of
**settings or {}handles both None and empty dictionary cases gracefully, maintaining backward compatibility.Consider adding basic validation for the settings dictionary to provide better error messages:
+ # Validate settings if provided + if settings: + # Basic validation - ensure all values are JSON serializable for logging + try: + import json + json.dumps(settings) + except (TypeError, ValueError) as e: + raise ValueError(f"Invalid settings provided: {e}") + # Create FastMCP server with the app's name mcp = FastMCP( name=app.name or "mcp_agent_server", # TODO: saqadri (MAC) - create a much more detailed description # based on all the available agents and workflows, # or use the MCPApp's description if available. instructions=f"MCP server exposing {app.name} workflows and agents. Description: {app.description}", lifespan=app_specific_lifespan, **settings or {}, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/mcp_agent_server/asyncio/README.md(2 hunks)examples/mcp_agent_server/asyncio/basic_agent_server.py(3 hunks)examples/mcp_agent_server/asyncio/client.py(3 hunks)src/mcp_agent/server/app_server.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/utils/config.py : Configuration values such as quality_threshold, max_refinement_attempts, consolidation_interval, and evaluator_model_provider must be loaded from mcp_agent.config.yaml.
📚 Learning: applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : use ...
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.
Applied to files:
examples/mcp_agent_server/asyncio/client.pyexamples/mcp_agent_server/asyncio/README.mdexamples/mcp_agent_server/asyncio/basic_agent_server.py
📚 Learning: applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/utils/config.p...
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/utils/config.py : Configuration values such as quality_threshold, max_refinement_attempts, consolidation_interval, and evaluator_model_provider must be loaded from mcp_agent.config.yaml.
Applied to files:
examples/mcp_agent_server/asyncio/basic_agent_server.py
🧬 Code Graph Analysis (1)
src/mcp_agent/server/app_server.py (1)
src/mcp_agent/app.py (1)
MCPApp(35-532)
🔇 Additional comments (11)
src/mcp_agent/server/app_server.py (2)
84-86: LGTM! Function signature updated correctly.The addition of the optional
settingsparameter follows good practices with proper typing and default value.
92-92: Docstring updated appropriately.The parameter documentation clearly explains the purpose of the new
settingsparameter.examples/mcp_agent_server/asyncio/README.md (3)
15-15: Documentation updated to reflect new capability.The mention of custom FastMCP settings is appropriately placed in the concepts section.
96-99: Clear server usage examples provided.The documentation shows both default and custom FastMCP settings usage patterns for the server.
105-108: Client usage examples included.The documentation consistently shows the flag usage for both server and client, which helps users understand the complete workflow.
examples/mcp_agent_server/asyncio/client.py (3)
1-1: Import added for command-line argument parsing.The
argparseimport is correctly placed and necessary for the new functionality.
15-23: Command-line argument parsing implemented correctly.The argument parser setup follows standard practices with a clear help message and appropriate action type.
34-45: Server arguments conditionally modified based on flag.The implementation correctly adjusts the server command arguments and includes informative logging to indicate which settings are being used.
examples/mcp_agent_server/asyncio/basic_agent_server.py (3)
10-10: Import added for command-line argument parsing.The
argparseimport is correctly placed and necessary for the new functionality.
173-181: Command-line argument parsing implemented correctly.The argument parser setup follows standard practices with a clear help message and appropriate action type.
195-204: Settings configuration and server creation updated appropriately.The conditional settings creation provides meaningful custom values for demonstration purposes. The settings are properly passed to the
create_mcp_server_for_appfunction, and logging is added to show the applied configuration.
src/mcp_agent/server/app_server.py
Outdated
| @@ -119,6 +122,7 @@ async def app_specific_lifespan(mcp: FastMCP) -> AsyncIterator[ServerContext]: | |||
| # or use the MCPApp's description if available. | |||
| instructions=f"MCP server exposing {app.name} workflows and agents. Description: {app.description}", | |||
| lifespan=app_specific_lifespan, | |||
| **settings or {}, | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
src/mcp_agent/server/app_server.py
Outdated
| def create_mcp_server_for_app( | ||
| app: MCPApp, settings: dict[str, Any] | None = None | ||
| ) -> FastMCP: |
There was a problem hiding this comment.
Should we do this simply as kwargs
| def create_mcp_server_for_app( | |
| app: MCPApp, settings: dict[str, Any] | None = None | |
| ) -> FastMCP: | |
| def create_mcp_server_for_app( | |
| app: MCPApp, **kwargs: Any | |
| ) -> FastMCP: |
There was a problem hiding this comment.
That works, too. Updated
Description
A recent regression in
mcp'sFastMCPintegration (fixed in modelcontextprotocol/python-sdk#1244) can be mitigated by allowing users to pass in explicit FastMCP settings throughcreate_mcp_server_for_app, rather than relying on environment variables that may or may not be used. Additionally, this allows for dynamic configuration of mcp app servers rather than having to rely on the environment variable being set.Changes
create_mcp_server_for_appto support a settings dict (note, FastMCP Settings type is not exported so we would need to redefine a subset if we wanted something typed).--custom-fastmcp-settingsflagTesting
Run with and without the flag set and ensure proper settings in the server, e.g.
Summary by CodeRabbit