Skip to content

Commit facca54

Browse files
Copilotjozkee
andcommitted
Address code review feedback: add Theme property, fix docs, improve test patterns
Co-authored-by: jozkee <16040868+jozkee@users.noreply.github.com>
1 parent 0cd7f87 commit facca54

11 files changed

Lines changed: 49 additions & 85 deletions

src/ModelContextProtocol.Core/Protocol/Icon.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,14 @@ public sealed class Icon
7272
/// </remarks>
7373
[JsonPropertyName("sizes")]
7474
public IList<string>? Sizes { get; init; }
75+
76+
/// <summary>
77+
/// Gets or sets the optional theme for this icon.
78+
/// </summary>
79+
/// <remarks>
80+
/// Can be "light", "dark", or a custom theme identifier.
81+
/// Used to specify which UI theme the icon is designed for.
82+
/// </remarks>
83+
[JsonPropertyName("theme")]
84+
public string? Theme { get; init; }
7585
}

src/ModelContextProtocol.Core/Protocol/Implementation.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public sealed class Implementation : IBaseMetadata
4141
/// Gets or sets an optional list of icons for this implementation.
4242
/// </summary>
4343
/// <remarks>
44-
/// This can be used by clients to display the implementation in a user interface.
44+
/// This can be used by clients to display the implementation's icon in a user interface.
4545
/// </remarks>
4646
[JsonPropertyName("icons")]
4747
public IList<Icon>? Icons { get; set; }

src/ModelContextProtocol.Core/Protocol/ResourceTemplate.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ public sealed class ResourceTemplate : IBaseMetadata
7373
public Annotations? Annotations { get; init; }
7474

7575
/// <summary>
76-
/// Gets or sets the icons for this resource template.
76+
/// Gets or sets an optional list of icons for this resource template.
7777
/// </summary>
7878
/// <remarks>
79-
/// This can be used by clients to display the resource's icon in a user interface.
79+
/// This can be used by clients to display the resource template's icon in a user interface.
8080
/// </remarks>
8181
[JsonPropertyName("icons")]
8282
public IList<Icon>? Icons { get; set; }

src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ private static McpServerToolCreateOptions DeriveOptions(MethodInfo method, McpSe
177177
newOptions.ReadOnly ??= readOnly;
178178
}
179179

180-
if (toolAttr.IconSource is { Length: > 0 } iconSource)
180+
if (newOptions.Icons is null && toolAttr.IconSource is { Length: > 0 } iconSource)
181181
{
182-
newOptions.Icons ??= [new() { Source = iconSource }];
182+
newOptions.Icons = [new() { Source = iconSource }];
183183
}
184184

185185
newOptions.UseStructuredContent = toolAttr.UseStructuredContent;

tests/ModelContextProtocol.Tests/Client/McpClientTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ public async Task CanReadServerInfo()
5858

5959
Assert.Equal("https://example.com/icon-48.png", serverInfo.Icons[0].Source);
6060
Assert.Equal("image/png", serverInfo.Icons[0].MimeType);
61-
var icon0Sizes = serverInfo.Icons[0].Sizes; Assert.NotNull(icon0Sizes); Assert.Contains("48x48", icon0Sizes);
61+
Assert.Single(serverInfo.Icons[0].Sizes, "48x48");
6262

6363
Assert.Equal("https://example.com/icon.svg", serverInfo.Icons[1].Source);
6464
Assert.Equal("image/svg+xml", serverInfo.Icons[1].MimeType);
65-
var icon1Sizes = serverInfo.Icons[1].Sizes; Assert.NotNull(icon1Sizes); Assert.Contains("any", icon1Sizes);
65+
Assert.Single(serverInfo.Icons[1].Sizes, "any");
6666
}
6767

6868
[Theory]

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsPromptsTests.cs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public async Task Can_Be_Notified_Of_Prompt_Changes()
166166
}
167167

168168
[Fact]
169-
public async Task TitleAttributeProperty_PropagatedToTitle()
169+
public async Task AttributeProperties_Propagated()
170170
{
171171
await using McpClient client = await CreateMcpClientForServer();
172172

@@ -177,18 +177,6 @@ public async Task TitleAttributeProperty_PropagatedToTitle()
177177
McpClientPrompt prompt = prompts.First(t => t.Name == "returns_string");
178178

179179
Assert.Equal("This is a title", prompt.Title);
180-
}
181-
182-
[Fact]
183-
public async Task IconSourceAttributeProperty_PropagatedToIcons()
184-
{
185-
await using McpClient client = await CreateMcpClientForServer();
186-
187-
var prompts = await client.ListPromptsAsync(cancellationToken: TestContext.Current.CancellationToken);
188-
Assert.NotNull(prompts);
189-
Assert.NotEmpty(prompts);
190-
191-
McpClientPrompt prompt = prompts.First(t => t.Name == "returns_string");
192180

193181
Assert.NotNull(prompt.ProtocolPrompt.Icons);
194182
Assert.NotEmpty(prompt.ProtocolPrompt.Icons);

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsResourcesTests.cs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public async Task Can_Be_Notified_Of_Resource_Changes()
201201
}
202202

203203
[Fact]
204-
public async Task TitleAttributeProperty_PropagatedToTitle()
204+
public async Task AttributeProperties_Propagated()
205205
{
206206
await using McpClient client = await CreateMcpClientForServer();
207207

@@ -211,22 +211,6 @@ public async Task TitleAttributeProperty_PropagatedToTitle()
211211
McpClientResource resource = resources.First(t => t.Name == "some_neat_direct_resource");
212212
Assert.Equal("This is a title", resource.Title);
213213

214-
var resourceTemplates = await client.ListResourceTemplatesAsync(cancellationToken: TestContext.Current.CancellationToken);
215-
Assert.NotNull(resourceTemplates);
216-
Assert.NotEmpty(resourceTemplates);
217-
McpClientResourceTemplate resourceTemplate = resourceTemplates.First(t => t.Name == "some_neat_templated_resource");
218-
Assert.Equal("This is another title", resourceTemplate.Title);
219-
}
220-
221-
[Fact]
222-
public async Task IconSourceAttributeProperty_PropagatedToIcons()
223-
{
224-
await using McpClient client = await CreateMcpClientForServer();
225-
226-
var resources = await client.ListResourcesAsync(cancellationToken: TestContext.Current.CancellationToken);
227-
Assert.NotNull(resources);
228-
Assert.NotEmpty(resources);
229-
McpClientResource resource = resources.First(t => t.Name == "some_neat_direct_resource");
230214
Assert.NotNull(resource.ProtocolResource.Icons);
231215
Assert.NotEmpty(resource.ProtocolResource.Icons);
232216
Assert.Equal("https://example.com/direct-resource-icon.svg", resource.ProtocolResource.Icons[0].Source);
@@ -235,6 +219,8 @@ public async Task IconSourceAttributeProperty_PropagatedToIcons()
235219
Assert.NotNull(resourceTemplates);
236220
Assert.NotEmpty(resourceTemplates);
237221
McpClientResourceTemplate resourceTemplate = resourceTemplates.First(t => t.Name == "some_neat_templated_resource");
222+
Assert.Equal("This is another title", resourceTemplate.Title);
223+
238224
Assert.NotNull(resourceTemplate.ProtocolResourceTemplate.Icons);
239225
Assert.NotEmpty(resourceTemplate.ProtocolResourceTemplate.Icons);
240226
Assert.Equal("https://example.com/templated-resource-icon.svg", resourceTemplate.ProtocolResourceTemplate.Icons[0].Source);

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ public void Create_ExtractsToolAnnotations_SomeSet()
630630
}
631631

632632
[Fact]
633-
public async Task TitleAttributeProperty_PropagatedToTitle()
633+
public async Task AttributeProperties_Propagated()
634634
{
635635
await using McpClient client = await CreateMcpClientForServer();
636636

@@ -643,18 +643,6 @@ public async Task TitleAttributeProperty_PropagatedToTitle()
643643
Assert.Equal("This is a title", tool.Title);
644644
Assert.Equal("This is a title", tool.ProtocolTool.Title);
645645
Assert.Equal("This is a title", tool.ProtocolTool.Annotations?.Title);
646-
}
647-
648-
[Fact]
649-
public async Task IconSourceAttributeProperty_PropagatedToIcons()
650-
{
651-
await using McpClient client = await CreateMcpClientForServer();
652-
653-
var tools = await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken);
654-
Assert.NotNull(tools);
655-
Assert.NotEmpty(tools);
656-
657-
McpClientTool tool = tools.First(t => t.Name == "echo_complex");
658646

659647
Assert.NotNull(tool.ProtocolTool.Icons);
660648
Assert.NotEmpty(tool.ProtocolTool.Icons);

tests/ModelContextProtocol.Tests/Server/McpServerPromptTests.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -506,22 +506,20 @@ public void SupportsIconsInCreateOptions()
506506
Icons = icons
507507
});
508508

509-
Assert.NotNull(prompt.ProtocolPrompt.Icons);
510-
Assert.Single(prompt.ProtocolPrompt.Icons);
511-
Assert.Equal("https://example.com/prompt-icon.png", prompt.ProtocolPrompt.Icons[0].Source);
512-
Assert.Equal("image/png", prompt.ProtocolPrompt.Icons[0].MimeType);
509+
var icon = Assert.Single(prompt.ProtocolPrompt.Icons);
510+
Assert.Equal("https://example.com/prompt-icon.png", icon.Source);
511+
Assert.Equal("image/png", icon.MimeType);
513512
}
514513

515514
[Fact]
516515
public void SupportsIconSourceInAttribute()
517516
{
518517
McpServerPrompt prompt = McpServerPrompt.Create([McpServerPrompt(IconSource = "https://example.com/prompt-icon.svg")] () => "test prompt");
519518

520-
Assert.NotNull(prompt.ProtocolPrompt.Icons);
521-
Assert.Single(prompt.ProtocolPrompt.Icons);
522-
Assert.Equal("https://example.com/prompt-icon.svg", prompt.ProtocolPrompt.Icons[0].Source);
523-
Assert.Null(prompt.ProtocolPrompt.Icons[0].MimeType);
524-
Assert.Null(prompt.ProtocolPrompt.Icons[0].Sizes);
519+
var icon = Assert.Single(prompt.ProtocolPrompt.Icons);
520+
Assert.Equal("https://example.com/prompt-icon.svg", icon.Source);
521+
Assert.Null(icon.MimeType);
522+
Assert.Null(icon.Sizes);
525523
}
526524

527525
[Fact]
@@ -537,10 +535,9 @@ public void CreateOptionsIconsOverrideAttributeIconSource_Prompt()
537535
Icons = optionsIcons
538536
});
539537

540-
Assert.NotNull(prompt.ProtocolPrompt.Icons);
541-
Assert.Single(prompt.ProtocolPrompt.Icons);
542-
Assert.Equal("https://example.com/override-icon.svg", prompt.ProtocolPrompt.Icons[0].Source);
543-
Assert.Equal("image/svg+xml", prompt.ProtocolPrompt.Icons[0].MimeType);
538+
var icon = Assert.Single(prompt.ProtocolPrompt.Icons);
539+
Assert.Equal("https://example.com/override-icon.svg", icon.Source);
540+
Assert.Equal("image/svg+xml", icon.MimeType);
544541
}
545542

546543
[Fact]

tests/ModelContextProtocol.Tests/Server/McpServerResourceTests.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -691,22 +691,20 @@ public void SupportsIconsInResourceCreateOptions()
691691
Icons = icons
692692
});
693693

694-
Assert.NotNull(resource.ProtocolResourceTemplate.Icons);
695-
Assert.Single(resource.ProtocolResourceTemplate.Icons);
696-
Assert.Equal("https://example.com/resource-icon.png", resource.ProtocolResourceTemplate.Icons[0].Source);
697-
Assert.Equal("image/png", resource.ProtocolResourceTemplate.Icons[0].MimeType);
694+
var icon = Assert.Single(resource.ProtocolResourceTemplate.Icons);
695+
Assert.Equal("https://example.com/resource-icon.png", icon.Source);
696+
Assert.Equal("image/png", icon.MimeType);
698697
}
699698

700699
[Fact]
701700
public void SupportsIconSourceInResourceAttribute()
702701
{
703702
McpServerResource resource = McpServerResource.Create([McpServerResource(UriTemplate = "test://resource", IconSource = "https://example.com/resource-icon.svg")] () => "test content");
704703

705-
Assert.NotNull(resource.ProtocolResourceTemplate.Icons);
706-
Assert.Single(resource.ProtocolResourceTemplate.Icons);
707-
Assert.Equal("https://example.com/resource-icon.svg", resource.ProtocolResourceTemplate.Icons[0].Source);
708-
Assert.Null(resource.ProtocolResourceTemplate.Icons[0].MimeType);
709-
Assert.Null(resource.ProtocolResourceTemplate.Icons[0].Sizes);
704+
var icon = Assert.Single(resource.ProtocolResourceTemplate.Icons);
705+
Assert.Equal("https://example.com/resource-icon.svg", icon.Source);
706+
Assert.Null(icon.MimeType);
707+
Assert.Null(icon.Sizes);
710708
}
711709

712710
[Fact]
@@ -722,10 +720,9 @@ public void CreateOptionsIconsOverrideAttributeIconSource_Resource()
722720
Icons = optionsIcons
723721
});
724722

725-
Assert.NotNull(resource.ProtocolResourceTemplate.Icons);
726-
Assert.Single(resource.ProtocolResourceTemplate.Icons);
727-
Assert.Equal("https://example.com/override-icon.svg", resource.ProtocolResourceTemplate.Icons[0].Source);
728-
Assert.Equal("image/svg+xml", resource.ProtocolResourceTemplate.Icons[0].MimeType);
723+
var icon = Assert.Single(resource.ProtocolResourceTemplate.Icons);
724+
Assert.Equal("https://example.com/override-icon.svg", icon.Source);
725+
Assert.Equal("image/svg+xml", icon.MimeType);
729726
}
730727

731728
[Fact]

0 commit comments

Comments
 (0)