-
Notifications
You must be signed in to change notification settings - Fork 110
Fix for #56 - missing method attribute from client RPC message #67
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
Fix for #56 - missing method attribute from client RPC message #67
Conversation
…rsor) for modelcontextprotocol#56 - method not present in RPC message params map
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/shared/TestTransport.kt:1
- The package name appears to be duplicated; consider changing it to 'io.modelcontextprotocol.kotlin.sdk.shared' to align with the directory structure and avoid confusion.
package io.modelcontextprotocol.kotlin.sdk.io.modelcontextprotocol.kotlin.sdk.shared
7637f9c
to
7006ce9
Compare
Fixed package for |
The odd thing is that the latest release seems to have somehow fixed this particular problem despite not including my PR or something like it. I haven't had a chance to debug it and find out what's happening, but it might be worth examining before merging this. |
Hmm, I will check, because in my case this issue still was present |
Yeah, seems to be fixed for me as well. There's another bug left but I will make a separate PR for it, I suppose.
|
Well done, @tomakehurst and @IlyaGulya ! |
Motivation and Context
This PR fixes issue #56 where some MCP clients (including Cursor) don't include the "method" attribute in the params map of their notification messages. The absence of this attribute caused NPEs during initialization. The solution adds the method parameter to the params object when missing, ensuring compatibility with various clients.
How Has This Been Tested?
I've implemented a test suite that covers all potential scenarios:
Breaking Changes
None
Types of changes
Checklist
Additional context
This PR builds upon the initial work by @tomakehurst to fix issue #56.
It retains the fix while adding test coverage and removing unintended version changes as requested by the reviewer.