feat(mcptest): extend test server with prompt and resource support #346
Conversation
|
""" WalkthroughThe changes introduce support for registering prompts and resources in both the MCP server and its test server. New methods and struct types are added to manage prompts and resources, including batch registration. Tests are updated and expanded to verify the new prompt and resource handling capabilities in the test server. Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mcptest/mcptest_test.go (1)
133-136: Add type assertion safety check.The type assertion
getResult.Messages[0].Content.(mcp.TextContent)could panic if the content is not of the expected type.- content := getResult.Messages[0].Content.(mcp.TextContent) - if content.Text != "Hello, John!" { - t.Errorf("Expected message content 'Hello, John!', got %q", content.Text) - } + content, ok := getResult.Messages[0].Content.(mcp.TextContent) + if !ok { + t.Fatalf("Expected TextContent, got %T", getResult.Messages[0].Content) + } + if content.Text != "Hello, John!" { + t.Errorf("Expected message content 'Hello, John!', got %q", content.Text) + }server/server.go (1)
384-411: Consider extracting capability initialization helper.The capability initialization logic for prompts (lines 386-397) duplicates the pattern used in
implicitlyRegisterResourceCapabilities(). Consider extracting this into a similar helper method for consistency.+func (s *MCPServer) implicitlyRegisterPromptCapabilities() { + s.capabilitiesMu.RLock() + if s.capabilities.prompts == nil { + s.capabilitiesMu.RUnlock() + + s.capabilitiesMu.Lock() + if s.capabilities.prompts == nil { + s.capabilities.prompts = &promptCapabilities{} + } + s.capabilitiesMu.Unlock() + } else { + s.capabilitiesMu.RUnlock() + } +} // AddPrompts registers multiple prompts at once func (s *MCPServer) AddPrompts(prompts ...ServerPrompt) { - s.capabilitiesMu.RLock() - if s.capabilities.prompts == nil { - s.capabilitiesMu.RUnlock() - - s.capabilitiesMu.Lock() - if s.capabilities.prompts == nil { - s.capabilities.prompts = &promptCapabilities{} - } - s.capabilitiesMu.Unlock() - } else { - s.capabilitiesMu.RUnlock() - } + s.implicitlyRegisterPromptCapabilities()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mcptest/mcptest.go(3 hunks)mcptest/mcptest_test.go(2 hunks)server/server.go(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
mcptest/mcptest_test.go (4)
mcptest/mcptest.go (1)
NewUnstartedServer(57-74)mcp/prompts.go (7)
Prompt(43-51)PromptArgument(61-69)GetPromptRequest(20-28)GetPromptResult(32-37)PromptMessage(84-87)Role(73-73)RoleUser(76-76)mcp/types.go (5)
Content(804-806)Params(160-160)Resource(619-634)ReadResourceRequest(558-567)ResourceContents(669-671)mcp/utils.go (1)
NewTextContent(199-204)
mcptest/mcptest.go (3)
server/server.go (5)
ServerTool(50-53)ServerPrompt(56-59)ServerResource(62-65)PromptHandlerFunc(38-38)ResourceHandlerFunc(32-32)mcp/prompts.go (1)
Prompt(43-51)mcp/types.go (1)
Resource(619-634)
server/server.go (3)
mcp/prompts.go (1)
Prompt(43-51)mcp/types.go (1)
Resource(619-634)examples/custom_context/main.go (1)
MCPServer(100-102)
🔇 Additional comments (10)
mcptest/mcptest_test.go (3)
14-14: Good rename for clarity.Renaming
TestServertoTestServerWithToolimproves clarity and follows the pattern of the new test functions.
81-137: Well-structured prompt test.The test comprehensively validates prompt functionality including registration, handler execution, and response verification. The test structure follows good practices with proper setup, execution, and teardown.
139-186: Comprehensive resource test.The test thoroughly validates resource functionality including registration, handler execution, and content verification. Good use of type assertions with proper error handling.
mcptest/mcptest.go (3)
24-25: Good structural extension.Adding
promptsandresourcesslices to the Server struct follows the established pattern and maintains consistency with the existingtoolsslice.
89-113: Well-implemented batch and single methods.The prompt and resource methods follow the same excellent pattern as the existing tool methods:
- Batch methods (
AddPrompts,AddResources) for multiple additions- Single methods (
AddPrompt,AddResource) as convenient wrappers- Consistent parameter patterns and struct creation
127-128: Proper integration in startup sequence.The addition of
AddPromptsandAddResourcescalls in theStartmethod correctly integrates the new functionality into the server startup process, maintaining consistency with the existingAddToolscall.server/server.go (4)
55-65: Excellent structural consistency.The new
ServerPromptandServerResourcestructs follow the same pattern asServerTool, maintaining consistency across the codebase and providing a clean way to pair resources/prompts with their handlers.
320-346: Well-refactored resource registration.The refactoring to support batch operations while maintaining single-item convenience methods is well-executed. The use of
implicitlyRegisterResourceCapabilities()and proper mutex handling ensures thread safety.
413-416: Clean single-item wrapper method.The refactored
AddPromptmethod correctly delegates to the batch method, maintaining the convenience API while leveraging the batch implementation.
463-476: Excellent capability initialization helper.The
implicitlyRegisterResourceCapabilities()method provides a clean, thread-safe way to handle lazy capability initialization using the double-checked locking pattern. This improves code reusability and consistency.
vasayxtx
force-pushed
the
extend-mcptest-server
branch
from
93daddb to
de0304a
Compare
There was a problem hiding this comment.
@vasayxtx Thank you for the PR. Can you check the comments left by coderabbit about deduplication using the implicitlyRegisterResourceCapabilities function and about the type assertion? They are valid concerns. Since you have already made the implicitlyRegisterResourceCapabilities function, using it everywhere where it is appropriate would be helpful. Otherwise, this PR is good to go.
Add support for prompts and resources to the mcptest server, including: - New methods to add prompts and resources to test servers - Batch methods for adding multiple prompts/resources at once - Test cases demonstrating prompt and resource functionality - Helper methods for resource capabilities registration
vasayxtx
force-pushed
the
extend-mcptest-server
branch
from
de0304a to
d36a6f5
Compare
|
@pottekkat coderabbit's comments are processed. Could you pls check the PR again? Thanks! |
Description
This PR enhances the
mcptest.Serverby adding comprehensive support for prompts and resources alongside the existing tools functionality.Type of Change
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests