Skip to content

Commit ddf1299

Browse files
authored
feat: Add session resource helper functions (#617)
* feat: add session resource helper functions - Add ErrSessionDoesNotSupportResources error constant - Implement AddSessionResource() helper for adding single resource - Implement AddSessionResources() helper for batch resource operations - Implement DeleteSessionResources() helper for removing resources - Auto-register resource capabilities with listChanged=true for session resources - Send notifications/resources/list_changed when resources are modified - Match session tools helper pattern for consistency * update * test: fix race condition in TestNotificationErrorHandling Use channel instead of shared variable to avoid race condition when capturing errors from goroutines in the notification error handling test * test: fix AddSessionResource assertions to validate state changes - Capture pre-call resource count before AddSessionResource - Assert resource is present and count increased after call - Validate listChanged behavior matches expectations - Fix auto-registration check to properly verify capabilities * test: fix assertion for non-existent resource deletion - Update test to assert no notification is sent when deleting non-existent resource - Replace expectation of notification with assertion that no notification occurs - Fail test if unexpected notification is received * test: properly validate global+session resource merging - Exercise server's ListResources and ReadResource handlers with session context - Assert global resources appear in merged list - Verify session resource overrides global resource with same URI - Confirm ReadResource uses session handler for overridden resources - Test that non-overridden global resources still use global handlers * fix: only send resource notification when actually deleting resources - Modified DeleteSessionResources to track if resources were actually deleted - Only send list_changed notification if something was removed - Fixed race condition in TestStreamableHTTP_SessionWithResources by protecting shared state - Added sync.Once to prevent multiple WaitGroup.Done() calls * Add RFC 3986 URI validation to AddSessionResources and optimize DeleteSessionResources - Validate Resource.URI is non-empty and conforms to RFC 3986 using url.ParseRequestURI - Return formatted errors for invalid or empty URIs instead of silently inserting them - Skip SetSessionResources call in DeleteSessionResources when no resources were actually deleted - Add comprehensive test for session resource overriding global resources
1 parent caa59b1 commit ddf1299

File tree

7 files changed

+1175
-12
lines changed

7 files changed

+1175
-12
lines changed

server/errors.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ var (
1313
ErrToolNotFound = errors.New("tool not found")
1414

1515
// Session-related errors
16-
ErrSessionNotFound = errors.New("session not found")
17-
ErrSessionExists = errors.New("session already exists")
18-
ErrSessionNotInitialized = errors.New("session not properly initialized")
19-
ErrSessionDoesNotSupportTools = errors.New("session does not support per-session tools")
20-
ErrSessionDoesNotSupportLogging = errors.New("session does not support setting logging level")
16+
ErrSessionNotFound = errors.New("session not found")
17+
ErrSessionExists = errors.New("session already exists")
18+
ErrSessionNotInitialized = errors.New("session not properly initialized")
19+
ErrSessionDoesNotSupportTools = errors.New("session does not support per-session tools")
20+
ErrSessionDoesNotSupportResources = errors.New("session does not support per-session resources")
21+
ErrSessionDoesNotSupportLogging = errors.New("session does not support setting logging level")
2122

2223
// Notification-related errors
2324
ErrNotificationNotInitialized = errors.New("notification channel not initialized")

server/session.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package server
33
import (
44
"context"
55
"fmt"
6+
"net/url"
67

78
"github.com/mark3labs/mcp-go/mcp"
89
)
@@ -460,3 +461,155 @@ func (s *MCPServer) DeleteSessionTools(sessionID string, names ...string) error
460461

461462
return nil
462463
}
464+
465+
// AddSessionResource adds a resource for a specific session
466+
func (s *MCPServer) AddSessionResource(sessionID string, resource mcp.Resource, handler ResourceHandlerFunc) error {
467+
return s.AddSessionResources(sessionID, ServerResource{Resource: resource, Handler: handler})
468+
}
469+
470+
// AddSessionResources adds resources for a specific session
471+
func (s *MCPServer) AddSessionResources(sessionID string, resources ...ServerResource) error {
472+
sessionValue, ok := s.sessions.Load(sessionID)
473+
if !ok {
474+
return ErrSessionNotFound
475+
}
476+
477+
session, ok := sessionValue.(SessionWithResources)
478+
if !ok {
479+
return ErrSessionDoesNotSupportResources
480+
}
481+
482+
// For session resources, we want listChanged enabled by default
483+
s.implicitlyRegisterCapabilities(
484+
func() bool { return s.capabilities.resources != nil },
485+
func() { s.capabilities.resources = &resourceCapabilities{listChanged: true} },
486+
)
487+
488+
// Get existing resources (this should return a thread-safe copy)
489+
sessionResources := session.GetSessionResources()
490+
491+
// Create a new map to avoid concurrent modification issues
492+
newSessionResources := make(map[string]ServerResource, len(sessionResources)+len(resources))
493+
494+
// Copy existing resources
495+
for k, v := range sessionResources {
496+
newSessionResources[k] = v
497+
}
498+
499+
// Add new resources with validation
500+
for _, resource := range resources {
501+
// Validate that URI is non-empty
502+
if resource.Resource.URI == "" {
503+
return fmt.Errorf("resource URI cannot be empty")
504+
}
505+
506+
// Validate that URI conforms to RFC 3986
507+
if _, err := url.ParseRequestURI(resource.Resource.URI); err != nil {
508+
return fmt.Errorf("invalid resource URI: %w", err)
509+
}
510+
511+
newSessionResources[resource.Resource.URI] = resource
512+
}
513+
514+
// Set the resources (this should be thread-safe)
515+
session.SetSessionResources(newSessionResources)
516+
517+
// It only makes sense to send resource notifications to initialized sessions --
518+
// if we're not initialized yet the client can't possibly have sent their
519+
// initial resources/list message.
520+
//
521+
// For initialized sessions, honor resources.listChanged, which is specifically
522+
// about whether notifications will be sent or not.
523+
// see <https://modelcontextprotocol.io/specification/2025-03-26/server/resources#capabilities>
524+
if session.Initialized() && s.capabilities.resources != nil && s.capabilities.resources.listChanged {
525+
// Send notification only to this session
526+
if err := s.SendNotificationToSpecificClient(sessionID, "notifications/resources/list_changed", nil); err != nil {
527+
// Log the error but don't fail the operation
528+
// The resources were successfully added, but notification failed
529+
if s.hooks != nil && len(s.hooks.OnError) > 0 {
530+
hooks := s.hooks
531+
go func(sID string, hooks *Hooks) {
532+
ctx := context.Background()
533+
hooks.onError(ctx, nil, "notification", map[string]any{
534+
"method": "notifications/resources/list_changed",
535+
"sessionID": sID,
536+
}, fmt.Errorf("failed to send notification after adding resources: %w", err))
537+
}(sessionID, hooks)
538+
}
539+
}
540+
}
541+
542+
return nil
543+
}
544+
545+
// DeleteSessionResources removes resources from a specific session
546+
func (s *MCPServer) DeleteSessionResources(sessionID string, uris ...string) error {
547+
sessionValue, ok := s.sessions.Load(sessionID)
548+
if !ok {
549+
return ErrSessionNotFound
550+
}
551+
552+
session, ok := sessionValue.(SessionWithResources)
553+
if !ok {
554+
return ErrSessionDoesNotSupportResources
555+
}
556+
557+
// Get existing resources (this should return a thread-safe copy)
558+
sessionResources := session.GetSessionResources()
559+
if sessionResources == nil {
560+
return nil
561+
}
562+
563+
// Create a new map to avoid concurrent modification issues
564+
newSessionResources := make(map[string]ServerResource, len(sessionResources))
565+
566+
// Copy existing resources except those being deleted
567+
for k, v := range sessionResources {
568+
newSessionResources[k] = v
569+
}
570+
571+
// Remove specified resources and track if anything was actually deleted
572+
actuallyDeleted := false
573+
for _, uri := range uris {
574+
if _, exists := newSessionResources[uri]; exists {
575+
delete(newSessionResources, uri)
576+
actuallyDeleted = true
577+
}
578+
}
579+
580+
// Skip no-op write if nothing was actually deleted
581+
if !actuallyDeleted {
582+
return nil
583+
}
584+
585+
// Set the resources (this should be thread-safe)
586+
session.SetSessionResources(newSessionResources)
587+
588+
// It only makes sense to send resource notifications to initialized sessions --
589+
// if we're not initialized yet the client can't possibly have sent their
590+
// initial resources/list message.
591+
//
592+
// For initialized sessions, honor resources.listChanged, which is specifically
593+
// about whether notifications will be sent or not.
594+
// see <https://modelcontextprotocol.io/specification/2025-03-26/server/resources#capabilities>
595+
// Only send notification if something was actually deleted
596+
if actuallyDeleted && session.Initialized() && s.capabilities.resources != nil && s.capabilities.resources.listChanged {
597+
// Send notification only to this session
598+
if err := s.SendNotificationToSpecificClient(sessionID, "notifications/resources/list_changed", nil); err != nil {
599+
// Log the error but don't fail the operation
600+
// The resources were successfully deleted, but notification failed
601+
if s.hooks != nil && len(s.hooks.OnError) > 0 {
602+
hooks := s.hooks
603+
go func(sID string, hooks *Hooks) {
604+
ctx := context.Background()
605+
hooks.onError(ctx, nil, "notification", map[string]any{
606+
"method": "notifications/resources/list_changed",
607+
"sessionID": sID,
608+
}, fmt.Errorf("failed to send notification after deleting resources: %w", err))
609+
}(sessionID, hooks)
610+
}
611+
}
612+
}
613+
614+
return nil
615+
}

0 commit comments

Comments
 (0)