-
Notifications
You must be signed in to change notification settings - Fork 57
Implement legacy SSE client transport #101
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
a403cc2
to
525389a
Compare
Should we attempt to unify this with the existing http transport? I certainly don't want to but... The spec outlines how it is possible for a client to detect which its connecting to so it can support either. Its done like this...
Can't say this would be "fun" but considering the spec outlines how, and there isn't an easy way for a dev to know which the server they are connecting to supports in many circumstances, maybe its worth seeing if the LOE is moderate. |
@stallent Yeah, that's a great idea! We should totally do that instead. |
Very excited for this direction, wondering what servers are you testing this against? Tried with https://mcp.zapier.com/mcp (SSE) and it won't work with or without streaming, fails with 405 method not allowed, where |
I assume you tried on this branch with the SSETransport? You probably did, just double checking. Happy to debug this later this afternoon after a few dull meetings. |
I believe so, but let me confirm again before you invest any time Update:I configured gmail zappier integration and using SSE without OAuth ![]() SSE client fails with 404 when trying to post against ![]() I'm hopeful that zapier implementation is working since I tested with with Another Update:On the bright side, I was finally able to test streaming working against vercel (followed steps here https://vercel.com/templates/next.js/model-context-protocol-mcp-with-next-js) |
Figured out the issue, but not sure the "right" way to fix. constructMessageURL is encoding a character it shouldn't be. Using the zapier example the endpoint message is this:
and the resulting components.url at end of constructMessageURL is encoding the ? to %3F. Lame. If you change the return line to this dumb solution return URL(string: "\(components.scheme ?? "")://\(components.host ?? "")\(path)") It fixes it, but hopefully @mattt can do a better fix. |
Ace solution Stephen, I was printing URLs but somehow missed this, thanks! |
this version sucks a little less. I'm sure there are things still needed but this isn't as dumb as the above. private func constructMessageURL(from path: String) -> URL? {
guard var baseEndpointComponents = URLComponents(url: endpoint, resolvingAgainstBaseURL: true) else { return nil }
guard var messageEndpointComponents = URLComponents(string: path) else { return nil }
// if the new path is a full url, return it.
if messageEndpointComponents.scheme != nil {
return messageEndpointComponents.url
}
return messageEndpointComponents.url(relativeTo: baseEndpointComponents.url)
} |
As discussed in #95 (comment)
See https://modelcontextprotocol.io/specification/2024-11-05/basic/transports#http-with-sse