-
Notifications
You must be signed in to change notification settings - Fork 532
Implement SEP-973: Icons and metadata support for Implementations, Resources, Tools, and Prompts #802
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
base: main
Are you sure you want to change the base?
Conversation
@MackinnonBuck 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: MackinnonBuck <[email protected]>
… Resource, Tool, and Prompt Co-authored-by: MackinnonBuck <[email protected]>
…onsistency Co-authored-by: MackinnonBuck <[email protected]>
tests/ModelContextProtocol.Tests/Protocol/ImplementationTests.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.Tests/Protocol/ResourceAndPromptIconTests.cs
Outdated
Show resolved
Hide resolved
…files Co-authored-by: MackinnonBuck <[email protected]>
Co-authored-by: MackinnonBuck <[email protected]>
Co-authored-by: MackinnonBuck <[email protected]>
Co-authored-by: MackinnonBuck <[email protected]>
Co-authored-by: jozkee <[email protected]>
Co-authored-by: MackinnonBuck <[email protected]>
Co-authored-by: MackinnonBuck <[email protected]>
/// </remarks> | ||
[JsonPropertyName("sizes")] | ||
public IList<string>? Sizes { get; init; } | ||
} No newline at end of 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.
@copilot MCP specification was updated (again) to include a theme
property, can you add that. For context: modelcontextprotocol/modelcontextprotocol#1584
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.
Added the Theme
property to Icon class per the updated specification. (commit TBD)
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.
Added the Theme
property to Icon class per the updated specification. (facca54)
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.
@copilot you also need to update the tests to exercise this new property. Please try to update existing tests only.
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.
Updated existing Icon tests to exercise the Theme property. The changes include:
- Added Theme to the full property round-trip test
- Added null assertion for Theme in the required-properties-only test
- Added Theme to the JSON property names validation test
(commit hash will be available after push)
src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.Tests/Server/McpServerPromptTests.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.Tests/Server/McpServerPromptTests.cs
Outdated
Show resolved
Hide resolved
tests/ModelContextProtocol.Tests/Server/McpServerPromptTests.cs
Outdated
Show resolved
Hide resolved
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.
@MackinnonBuck do you think all these new tests added to Protocol folder are necessary?
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.
They're probably lower value than many of the other tests, and possibly not necessary, although they do validate that types are correctly configured for JSON serialization. Might be nice to catch any regressions that cause these types to get serialized differently.
Similar tests already exist for other parts of the protocol (e.g., ContentBlockTests
), so I had asked copilot to generate similar tests for protocol types touched by this PR.
If you feel the value provided by these tests doesn't make up for the cost of running or maintaining them, then we can consider removing them 🙂
…est patterns Co-authored-by: jozkee <[email protected]>
Co-authored-by: jozkee <[email protected]>
SEP-973: Icons and metadata support implementation
This PR implements SEP-973 which adds support for icons and additional metadata for Implementations, Resources, Tools, and Prompts.
Implementation Plan
Recent Changes (Addressing Latest Code Review Feedback)
Theme
property toIcon
class per specification update (PR #1584)Assert.Single()
pattern for cleaner, more idiomatic testsAIFunctionMcpServerTool
icon handling consistent with Prompt and Resource patternsTitleAttributeProperty_PropagatedToTitle
andIconSourceAttributeProperty_PropagatedToIcons
tests into singleAttributeProperties_Propagated
testsKey Changes Implemented
1. New Icon Class (
Icon.cs
)Source
property for URI pointing to icon resource (serialized as "src" in JSON)MimeType
,Sizes
, andTheme
propertiesIList<string>?
to support multiple size specifications (e.g.,["48x48", "96x96"]
)string?
to specify UI theme ("light", "dark", or custom identifiers)Source
property2. Enhanced Classes with Icon Support
Icons
andWebsiteUrl
propertiesIcons
propertyIcons
propertyIcons
propertyIcons
property3. Server-Side Tooling Integration
Tools:
Icons
property for programmatic multi-icon configurationIconSource
property for simple attribute-based single-icon configurationPrompts:
Icons
property for programmatic multi-icon configurationIconSource
property for simple attribute-based single-icon configurationResources:
Icons
property for programmatic multi-icon configurationIconSource
property for simple attribute-based single-icon configurationCommon Features:
4. Specification Compliance
✅ Matches TypeScript schema exactly
✅ Correct JSON property names (
icons
,websiteUrl
,src
,mimeType
,sizes
,theme
)✅ All new properties are optional for backward compatibility
✅ Supports multiple icons per entity
✅ Supports multiple size specifications per icon (as string array)
✅ Supports theme specification per icon (light, dark, or custom)
✅ Documents required (PNG, JPEG) and recommended (SVG, WebP) MIME types
✅ Includes security considerations for SVG and URI validation
✅ Uses .NET naming conventions while preserving JSON compatibility
5. Comprehensive Test Coverage
IconTests.cs
: Icon serialization, round-trip validation, JsonException tests, and Theme property validationImplementationTests.cs
: Full Implementation testing with required property validationToolTests.cs
: Tool functionality testing including new icon supportResourceTests.cs
: Resource serialization and property validationPromptTests.cs
: Prompt functionality testingMcpServerToolTests.cs
: Server-side Tool icon support using idiomatic test patternsMcpServerPromptTests.cs
: Server-side Prompt icon support via options and attributesMcpServerResourceTests.cs
: Server-side Resource icon support via options and attributesMcpServerBuilderExtensionsToolsTests
: Client-server test verifying tool icons retrieval and attribute propagationMcpServerBuilderExtensionsPromptsTests
: Client-server test verifying prompt icons retrieval and attribute propagationMcpServerBuilderExtensionsResourcesTests
: Client-server test verifying resource icons retrieval and attribute propagationMcpClientTests
: Verifies server Implementation icons and WebsiteUrlAssert.Single()
pattern for cleaner assertionsUsage Examples
Simple Attribute-Based Icon Configuration:
For Tools:
For Prompts:
For Resources:
Advanced Multi-Icon Configuration with Theme:
Server Implementation with Icons and WebsiteUrl:
Technical Details
IList<Icon>?
for consistency with existing collection patternsIList<string>?
forIcon.Sizes
to support multiple size specifications per the updated specstring?
forIcon.Theme
to support theme identifiersJsonPropertyName
attributesSecurity Features
Implementation includes documentation for:
Note: Build validation requires .NET 9 SDK. The implementation follows established patterns and should compile correctly with the appropriate SDK version.
Fixes #753.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.