Update MCP parser to support latest specification #1993
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1993 +/- ##
==========================================
+ Coverage 48.17% 48.29% +0.12%
==========================================
Files 233 233
Lines 29229 29297 +68
==========================================
+ Hits 14082 14150 +68
- Misses 14111 14116 +5
+ Partials 1036 1031 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the MCP parser to support the latest MCP specification (2025-06-18) by adding support for new methods and improving existing functionality. The implementation provides comprehensive coverage of all new MCP methods including elicitation, sampling, resource subscription, template listing, and various notification types.
Key Changes
- New method handlers: Added support for 11 new MCP methods including elicitation/create, sampling/createMessage, resource subscription operations, and notification handling
- Enhanced completion handler: Improved to support both PromptReference and ResourceTemplateReference types according to the latest schema
- Static resource mapping: Added static resource IDs for notification methods that don't require parameter parsing
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/mcp/parser.go | Core parser implementation with new method handlers and static resource mappings |
| pkg/mcp/parser_test.go | Comprehensive test coverage for all new methods and edge cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Fallback to string ref (legacy support) | ||
| if ref, ok := paramsMap["ref"].(string); ok { | ||
| return ref, nil | ||
| return ref, paramsMap |
There was a problem hiding this comment.
The legacy string ref case should return nil for arguments to maintain consistency with the original implementation. The change from returning nil to paramsMap could break existing code that expects nil when only extracting the resource ID.
| return ref, paramsMap | |
| return ref, nil |
Copilot uses AI. Check for mistakes.
| if token, ok := paramsMap["progressToken"].(float64); ok { | ||
| return fmt.Sprintf("%.0f", token), paramsMap | ||
| } |
There was a problem hiding this comment.
Using fmt.Sprintf("%.0f", token) could produce unexpected results for large numbers due to scientific notation. Consider using strconv.FormatFloat(token, 'f', 0, 64) for more predictable string conversion of numeric tokens.
Copilot uses AI. Check for mistakes.
| if requestId, ok := paramsMap["requestId"].(float64); ok { | ||
| return fmt.Sprintf("%.0f", requestId), paramsMap | ||
| } |
There was a problem hiding this comment.
Using fmt.Sprintf("%.0f", requestId) could produce unexpected results for large numbers due to scientific notation. Consider using strconv.FormatFloat(requestId, 'f', 0, 64) for more predictable string conversion of numeric request IDs.
Copilot uses AI. Check for mistakes.
pkg/mcp/parser.go
Outdated
| // handleSamplingMethod extracts resource ID for sampling/createMessage requests | ||
| func handleSamplingMethod(paramsMap map[string]interface{}) (string, map[string]interface{}) { | ||
| // Use model preferences or system prompt as identifier if available | ||
| if modelPrefs, ok := paramsMap["modelPreferences"].(map[string]interface{}); ok { |
There was a problem hiding this comment.
we are not checking here about the param existing, being nil, etc... are we sure that it will always contains the expected values?
| } | ||
| } | ||
| if systemPrompt, ok := paramsMap["systemPrompt"].(string); ok && systemPrompt != "" { | ||
| // Use first 50 chars of system prompt as identifier |
There was a problem hiding this comment.
why that number, is that defined somewhere? also... it's a prompt, is it ok to have it as a resource identifier? can contain special chars, etc... shouldn't be better to use a hash for it?
pkg/mcp/parser.go
Outdated
| return "", nil | ||
| } | ||
|
|
||
| // handleCompletionMethod extracts resource ID for completion requests |
There was a problem hiding this comment.
generally it would be good to document why we return the resource ID with different criteria for all those different methods
Add support for new MCP methods from the 2025-06-18 specification: - elicitation/create for user input requests - sampling/createMessage for message creation - resources/subscribe and resources/unsubscribe for resource subscriptions - resources/templates/list for template listing - roots/list for listing roots - notifications/progress for progress notifications - notifications/cancelled for cancellation notifications - Additional notification methods for list changes Update completion/complete handler to properly handle both PromptReference and ResourceTemplateReference types as specified in the latest schema. Add comprehensive test coverage for all new methods to ensure proper extraction of resource IDs and arguments.
- Added tests for all new MCP methods from 2025-06-18 specification - Added edge case tests for error handling and malformed inputs - Added tests for JSON-RPC notifications (messages without ID) - Improved test coverage from ~66% to 91.7% for pkg/mcp - All parser functions now have 83-100% coverage
Signed-off-by: Juan Antonio Osorio <[email protected]>
JAORMX
force-pushed
the
feat/update-mcp-parser-latest-spec
branch
from
1b588f7 to
8cf1ae5
Compare
Summary
This PR updates the MCP parser to support all methods from the latest MCP specification (2025-06-18).
Changes
New Methods Added
Improvements
completion/completehandler to properly handle bothPromptReferenceandResourceTemplateReferencetypes as specified in the latest schemaTesting
Testing Done
go test ./pkg/mcp/...)golangci-lint run --fix ./pkg/mcp/...)References