-
Notifications
You must be signed in to change notification settings - Fork 289
feat(mcp): add a feature to start MCP server from config file #623
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?
Conversation
Can we also add this to agent-builder? :) |
Agent-builder is specific for building an agent? This feature is to start a MCP ? |
server_params = StdioServerParameters(command=self.command, args=self.args, env=self.env) | ||
return stdio_client(server_params) | ||
|
||
return MCPClient(transport_callable) |
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.
Is this something that should live in the SDK - e.g. should Strands consumers be able to load MCP client from file? @dbschmigelski is this on your roadmap?
…okens to prevent unrecoverable state (strands-agents#607)
return str(config_path) | ||
|
||
|
||
class TestMCPTransportType: |
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.
Switch to top-level methods for tests; AFAIK that's the standard throughout the SDK
|
||
|
||
class MCPServerConfig: | ||
"""Configuration for an MCP server following MCP standards.""" |
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.
Is this truly a standard or a De facto standard? If the former, then let's link to the standard, if the latter let's document what the format is (both in code and in our docs) to make it obvious to readers/customers
or server_config.get("transportType") | ||
or server_config.get("transport_type") | ||
) | ||
if raw_transport is None and "url" in server_config: |
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.
Are there any other config formats that support streamable http or sse? If so, do they use the same logic for deciding on the format? If not, let's make it a hard requirement that you specify the transport type for non stdio formats. I'd rather be safe than sorry.
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.
This is prettey standard and firm. url end with SSE is SSE, end with MCP is Http.
eg: https://mcp.intercom.com/mcp | https://mcp.linear.app/sse
Stdio won't have url but must have command
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.
We should have an e2e integ test for this as well.
And we should have a docs PR that shows how this is to be used? The example of how it will be used should also be in the PR description for easy review
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.
Yes, sir. I will update PR accordingly.
Doc example in separate PR.
return [] | ||
|
||
@classmethod | ||
def _parse_mcp_servers_format(cls, mcp_servers: Dict[str, Dict]) -> List["MCPServerConfig"]: |
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.
Are we reading and validating json here? If so, would it make sense to model this using something like pydantic, and then use pydantic to validate the shape?
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.
It's a good idea but I think the only MUST is command
for STDIO and url
for Streamable-HTTP(sse/http), so we could keep it simple here.
Team deceided that we will have new MCP interface in next couple weeks. Will follow up this PR after release. |
Description
add a feature to start MCP server from config file
Anthropic MCP Server Config Examples
MCP TransportType Source Code
For STDIO, configuration file should like, only name, command is required:
For Streamable-http, only name, url is required:
Example:
Related Issues
Documentation PR
Type of Change
Bug fix
New feature
Breaking change
Documentation update
Other (please describe):
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
I ran unit tests and also ran an example from Anthropic
hatch run prepare
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.