-
Notifications
You must be signed in to change notification settings - Fork 132
mcp: add resource subscriptions #138
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
mcp: add resource subscriptions #138
Conversation
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.
Pull Request Overview
This PR implements resource subscriptions for the MCP (Model Context Protocol), allowing clients to subscribe to and receive notifications when specific resources change. The implementation follows the MCP specification for resource subscriptions.
Key changes:
- Added server-side subscription management with handlers for subscribe/unsubscribe operations
- Implemented client-side subscription methods and notification handling
- Added comprehensive test coverage for the subscription workflow
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
mcp/protocol.go | Defines subscription-related protocol structures (SubscribeParams, UnsubscribeParams, ResourceUpdatedNotificationParams) |
mcp/server.go | Implements server-side subscription tracking, handlers, and notification routing |
mcp/client.go | Adds client-side subscription methods and notification handler registration |
mcp/server_test.go | Tests server capability reporting for resource subscriptions |
mcp/mcp_test.go | End-to-end integration tests for the complete subscription workflow |
design/design.md | Documentation updates explaining subscription usage patterns |
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.
Nice.
7b5ea37
to
4d65676
Compare
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.
Thought for a followup: what if the inner map was map[*ServerSession]bool
?
Then we wouldn't need the loop to collect the sessions, the map keys would be the sessions.
mcp/server.go
Outdated
} | ||
var sessions []*ServerSession | ||
for _, session := range s.sessions { | ||
if _, ok := subscribedSessionIDs[session.ID()]; ok { |
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 a bool map, so just if m[k]
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.
Done
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.
Changed to map[*ServerSession]bool
This CL adds the ability for clients to subscribe and receive updates for resources as described in https://modelcontextprotocol.io/specification/2025-06-18/server/resources#subscriptions
Co-authored-by: Copilot <[email protected]>
1a96f92
to
14c9442
Compare
This CL adds the ability for clients to subscribe and receive updates for resources as described in https://modelcontextprotocol.io/specification/2025-06-18/server/resources#subscriptions
Fixes: #23