-
Notifications
You must be signed in to change notification settings - Fork 716
enhc(server): allow adding multiple resource templates at once #483
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
Conversation
Following the same pattern from tools, prompts and resources, this patch allows adding multiple resource templates at once.
|
""" WalkthroughThis change removes the local definition of the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsserver/server_test.go (2)🔇 Additional comments (2)
✨ 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 (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mcptest/mcptest.go(3 hunks)server/server.go(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.
server/server.go (1)
Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.
mcptest/mcptest.go (4)
Learnt from: octo
PR: mark3labs/mcp-go#149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Learnt from: ezynda3
PR: mark3labs/mcp-go#461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.
Learnt from: floatingIce91
PR: mark3labs/mcp-go#401
File: server/server.go:1082-1092
Timestamp: 2025-06-23T11:10:42.948Z
Learning: In Go MCP server, ServerTool.Tool field is only used for tool listing and indexing, not for tool execution or middleware. During handleToolCall, only the Handler field is used, so dynamic tools don't need the Tool field populated.
Learnt from: xinwo
PR: mark3labs/mcp-go#35
File: mcp/tools.go:107-137
Timestamp: 2025-03-04T06:59:43.882Z
Learning: Tool responses from the MCP server shouldn't contain RawInputSchema, which is why the UnmarshalJSON method for the Tool struct is implemented to handle only the structured InputSchema format.
🧬 Code Graph Analysis (1)
mcptest/mcptest.go (1)
server/server.go (1)
ServerResourceTemplate(68-71)
🔇 Additional comments (6)
server/server.go (3)
67-71: Well-designed struct following established patterns.The
ServerResourceTemplatestruct correctly follows the same pattern as other Server* types (ServerTool,ServerPrompt,ServerResource) by combining the MCP type with its handler function.
369-387: Excellent implementation of batch resource template registration.The
AddResourceTemplatesmethod correctly follows the established pattern from other batch operations:
- Proper capability registration
- Thread-safe locking
- Notification sending when
listChangedis enabled- Consistent with
AddResourcesandAddToolsimplementations
389-395: Good refactoring to use the batch method.Converting
AddResourceTemplateto useAddResourceTemplatesas a convenience wrapper maintains API compatibility while leveraging the new batch functionality. This pattern is consistent with other single-item methods in the codebase.mcptest/mcptest.go (3)
26-26: Correct usage of the centralized ServerResourceTemplate type.The field type has been properly updated to use
server.ServerResourceTemplate, aligning with the centralized resource template handling.
112-116: Proper construction of the server type.The method correctly constructs a
server.ServerResourceTemplatestruct with the appropriate field mapping (TemplateandHandler).
119-119: Signature correctly updated for the centralized type.The method signature now properly uses
server.ServerResourceTemplateparameter, maintaining consistency with the centralized approach.
|
@rafaeljusto can you add 1 or 2 tests for the new functions you added? After that should be gtg |
Add a test case to cover the new function. mark3labs#483 (comment)
|
@ezynda3 Sorry for the delay, added the tests now. 🙂 |
Description
Following the same pattern from tools, prompts and resources, this patch allows adding multiple resource templates at once.
Type of Change
Checklist
MCP Spec Compliance
Additional Information
The tests were already added by #449 .
Summary by CodeRabbit
New Features
Refactor