Skip to content

feat: add "theme" to Icon#766

Open
glicht wants to merge 1 commit intomodelcontextprotocol:mainfrom
binahm:feat/icon-theme
Open

feat: add "theme" to Icon#766
glicht wants to merge 1 commit intomodelcontextprotocol:mainfrom
binahm:feat/icon-theme

Conversation

@glicht
Copy link
Contributor

@glicht glicht commented Mar 22, 2026

Motivation and Context

Add the "theme" field to Icon struct as defined in the MCP Spec: https://modelcontextprotocol.io/specification/2025-11-25/schema#icon . Clients and Servers can use this field now.

How Has This Been Tested?

Updated unit tests to include the theme field. All tests pass.

Breaking Changes

No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@glicht glicht requested a review from a team as a code owner March 22, 2026 09:29
@github-actions github-actions bot added T-test Testing related changes T-config Configuration file changes T-core Core library changes T-model Model/data structure changes labels Mar 22, 2026
Copy link
Member

@DaleSeo DaleSeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this, @glicht! I have a couple of minor comments.

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Copy)]
#[serde(rename_all = "lowercase")] //match spec
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub enum IconTheme {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MCP spec could add more themes in the future (e.g., high-contrast). This would prevent downstream match arms from breaking when new variants are added.

Suggested change
pub enum IconTheme {
#[non_exhaustive]
pub enum IconTheme {

}

/// Icon themes supported by the MCP specification
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Copy)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding these would let consumers use it as a HashMap key or in HashSet without friction.

Suggested change
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Copy)]
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, Hash, Copy)]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-config Configuration file changes T-core Core library changes T-model Model/data structure changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants