-
Notifications
You must be signed in to change notification settings - Fork 24
fix(jetsocat): make the MCP proxy non intrusive #1514
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
b0096fb to
d3ddc1f
Compare
033e420 to
3e5a528
Compare
6e01dc8 to
185d843
Compare
The MCP proxy was previously handling the initialize request itself and returning a response instead of forwarding it to the target MCP server. It also intercepted notifications/initialized and logging/ setLevel without passing them through. Additionally, any unknown methods were being answered with an error response, even though the actual server might support them. This change forwards all requests and notifications directly to the server to ensure correct behavior and compatibility.
| if let Some(text) = item_obj.get_mut("text") { | ||
| if let Some(s) = text.get::<String>() { | ||
| let decoded = s | ||
| .replace("\\u0027", "'") |
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.
That may be the biggest if chain I've ever seen 😄
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.
Yeah right! 😂
This was a remaining of the AI-generated stuff from Marc-André’s repo: https://github.com/awakecoding/mcp-proxy-tool/blob/5c8e1664c06c2e0fea2c5f5e861788bd188adfe6/src/main.rs#L285-L306
But got worse when I removed serde-json
| [dependencies] | ||
| anyhow = "1.0" | ||
| assert_cmd = "2.0" | ||
| dynosaur = "0.3" |
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.
Wow, didn't know about that cool crate!
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 actually recommended over async-trait now! The implementer does not need to know about the crate too 🙂
pacmancoder
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.
LGTM!
The MCP proxy was previously handling the initialize request itself and returning a response instead of forwarding it to the target MCP server. It also intercepted notifications/initialized and logging/ setLevel without passing them through.
Additionally, any unknown methods were being answered with an error response, even though the actual server might support them.
This change forwards all requests and notifications directly to the server to ensure correct behavior and compatibility.
Issue: DGW-308