-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add sse mcp support #1495
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
Conversation
|
|
The pull request is quite large, with 4966 lines added and 3323 lines removed. While the changes in |
|
Awesome! Excited to take a look at this. |
c0b1d12 to
b4f808d
Compare
|
@mrubens I want this.. this is a big big deal. |
|
Looks good other than my one comment. Nice work! |
|
I merged main into your branch, hope that's ok. |
|
Do you think we should update the system prompt instructions around MCP to tell it about SSE support and the type field? https://github.com/RooVetGit/Roo-Code/blob/main/src/core/prompts/sections/mcp-servers.ts |
package.json
Outdated
| "delay": "^6.0.0", | ||
| "diff": "^5.2.0", | ||
| "diff-match-patch": "^1.0.5", | ||
| "eventsource": "^3.0.5", |
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.
Can I recommend using https://www.npmjs.com/package/reconnecting-eventsource instead?
I've had a lot of issues with MCP SSE stability when implementing it in Glama.
reconnecting-eventsource helped a lot with that.
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 really a good suggestion, i'll try it later.
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.
@punkpeye I have replaced reconnecting-eventsource with eventsource, thank you for your suggestion.
src/services/mcp/McpHub.ts
Outdated
| } | ||
| } | ||
|
|
||
| private async handleSSEReconnection(name: string, config: z.infer<typeof ServerConfigSchema>): Promise<void> { |
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.
Adding reconnecting-eventsource would get rid of some of this boilerplate code too.
|
@aheizi after seeing this PR, I realized that the ecosystem would benefit from a higher level abstraction for interacting with MCP, and I built https://github.com/punkpeye/mcp-client. Would you consider using it for this PR? |
ok |
@mrubens done |
8e6927c to
f534e80
Compare
I thought about it and think this is another matter; we can first use the native @mrubens How do you think about it? |
Yeah that sounds fine to me. Excited to get this in. I just tried to resolve the conflicts. |
mrubens
left a comment
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.
Looks good!
Uncomment i18n Feature + Add to Welcome Page
Context
support sse mcp
Implementation
use
@modelcontextprotocol/sdkSSEClientTransportScreenshots
How to Test
step1:
here is a simple SSE MCP server : https://github.com/aheizi/simple-sse-mcp, you can start it yourself
or you can use https://mcp.composio.dev/
step2:
update
cline_mcp_settings.jsonNow you can use this SSE MCP.
Get in Touch
aheizi
Important
Adds SSE MCP support in
McpHub.tswithSSEClientTransport, error handling, and reconnection logic.SSEClientTransportinMcpHub.ts.connectToServer()andhandleSSEReconnection().McpSettingsSchemato include SSE configuration.@modelcontextprotocol/sdkto^1.5.0and addseventsourcetopackage.json.SSEConfigSchemaandServerConfigSchemainMcpHub.tsto handle both Stdio and SSE configurations.This description was created by
for bb42fb6. It will automatically update as commits are pushed.