Add ConformanceServer sample #983
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new ConformanceServer sample that implements the MCP specification for testing against the MCP Conformance test suite. The server demonstrates all major MCP capabilities including tools, resources, prompts, sampling, elicitation, subscriptions, and logging. Testing shows 29 out of 30 conformance tests pass, with the single failure being a test configuration issue.
Key changes:
- Implements comprehensive tool handlers for testing different content types, logging, progress, error handling, sampling, and elicitation (including SEP-1034 and SEP-1330)
- Adds resource handlers for static text/binary content, templates with parameters, and subscribable resources
- Provides prompt handlers demonstrating simple, parameterized, embedded resource, and image-based prompts
- Includes subscription management and completion support
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| samples/ComplianceServer/ComplianceServer.csproj | Defines the project structure targeting .NET 9.0 with references to ModelContextProtocol.AspNetCore |
| samples/ComplianceServer/Program.cs | Configures the MCP server with HTTP transport, registers handlers for tools/prompts/resources, and implements subscription management |
| samples/ComplianceServer/Tools/ComplianceTools.cs | Implements 12 test tools covering text/image/audio content, logging, progress, errors, sampling, and elicitation with SEP-1034/SEP-1330 support |
| samples/ComplianceServer/Resources/ComplianceResources.cs | Provides static text/binary resources and a parameterized template resource for conformance testing |
| samples/ComplianceServer/Prompts/CompliancePrompts.cs | Implements simple, parameterized, embedded resource, and image-based prompts |
| samples/ComplianceServer/appsettings.json | Standard ASP.NET Core configuration for logging and allowed hosts |
| samples/ComplianceServer/appsettings.Development.json | Development-specific logging configuration |
| samples/ComplianceServer/Properties/launchSettings.json | Defines HTTP and HTTPS launch profiles for localhost:3001 |
| samples/ComplianceServer/ComplianceServer.http | Provides sample HTTP requests for testing the server endpoints |
| ModelContextProtocol.slnx | Adds the ComplianceServer project to the solution |
Comments suppressed due to low confidence (1)
samples/ComplianceServer/Tools/ComplianceTools.cs:1
- Field names in result content access don't match the schema property names defined above. The code accesses 'stringField', 'numberField', and 'booleanField', but the schema defines 'name', 'age', 'score', 'status', and 'verified'.
using ModelContextProtocol;
tests/ModelContextProtocol.ConformanceTests/ConformanceServer/ConformanceServer.csproj
Show resolved
Hide resolved
| public static IEnumerable<PromptMessage> PromptWithEmbeddedResource( | ||
| [Description("URI of the resource to embed")] string resourceUri) | ||
| { | ||
| return [ |
There was a problem hiding this comment.
Similarly here, you could return a PromptMessage instead of an enumerable of prompt messages, if you want.
e928861 to
550cdc4
Compare
|
@stephentoub I think I've addressed all your comments. Copilot wrote the test class -- I'm not sure it is right so please give that a close look. |
| return $"Accepted with values: string={result.Content["stringField"].GetString()}, " + | ||
| $"number={result.Content["numberField"].GetInt32()}, " + | ||
| $"boolean={result.Content["booleanField"].GetBoolean()}"; |
There was a problem hiding this comment.
Field name mismatch in result.Content access. The schema defines properties "name", "age", and "verified", but the code attempts to access "stringField", "numberField", and "booleanField". This will cause a runtime error. Should be:
return $"Accepted with values: string={result.Content["name"].GetString()}, " +
$"number={result.Content["age"].GetInt32()}, " +
$"boolean={result.Content["verified"].GetBoolean()}";| return $"Accepted with values: string={result.Content["stringField"].GetString()}, " + | |
| $"number={result.Content["numberField"].GetInt32()}, " + | |
| $"boolean={result.Content["booleanField"].GetBoolean()}"; | |
| return $"Accepted with values: string={result.Content["name"].GetString()}, " + | |
| $"number={result.Content["age"].GetInt32()}, " + | |
| $"boolean={result.Content["verified"].GetBoolean()}"; |
| } | ||
| if (ctx.Params?.Uri is { } uri) | ||
| { | ||
| subscriptions[ctx.Server.SessionId].TryAdd(uri, 0); |
There was a problem hiding this comment.
Potential KeyNotFoundException when accessing subscriptions[ctx.Server.SessionId]. The code doesn't ensure that an entry exists for the session ID before attempting to add to it. Should use GetOrAdd to ensure the inner dictionary exists:
subscriptions.GetOrAdd(ctx.Server.SessionId, _ => new ConcurrentDictionary<string, byte>()).TryAdd(uri, 0);| subscriptions[ctx.Server.SessionId].TryAdd(uri, 0); | |
| subscriptions.GetOrAdd(ctx.Server.SessionId, _ => new ConcurrentDictionary<string, byte>()).TryAdd(uri, 0); |
| } | ||
| if (ctx.Params?.Uri is { } uri) | ||
| { | ||
| subscriptions[ctx.Server.SessionId].TryRemove(uri, out _); |
There was a problem hiding this comment.
Potential KeyNotFoundException when accessing subscriptions[ctx.Server.SessionId]. The code doesn't check if an entry exists for the session ID before attempting to remove from it. Should use TryGetValue:
if (subscriptions.TryGetValue(ctx.Server.SessionId, out var sessionSubscriptions))
{
sessionSubscriptions.TryRemove(uri, out _);
}| subscriptions[ctx.Server.SessionId].TryRemove(uri, out _); | |
| if (subscriptions.TryGetValue(ctx.Server.SessionId, out var sessionSubscriptions)) | |
| { | |
| sessionSubscriptions.TryRemove(uri, out _); | |
| } |
| // Report the results | ||
| Assert.True(result.Success, | ||
| $"Conformance tests failed.\n\nStdout:\n{result.Output}\n\nStderr:\n{result.Error}"); | ||
| } private static Process StartConformanceServer() |
There was a problem hiding this comment.
[nitpick] Missing blank line between methods. The closing brace of RunConformanceTests() on line 79 should be followed by a blank line before the StartConformanceServer() method declaration to improve readability.
| } private static Process StartConformanceServer() | |
| } | |
| private static Process StartConformanceServer() |
| var startInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "dotnet", | ||
| Arguments = $"run --no-build --project ConformanceServer.csproj --urls {ServerUrl}", |
There was a problem hiding this comment.
The --no-build flag may cause test failures if the ConformanceServer hasn't been built prior to running tests. While the ProjectReference should ensure it's built when the test project builds, it's safer to remove --no-build to ensure the server is always built with the latest changes before running tests, especially in CI/CD scenarios or when running tests individually.
| Arguments = $"run --no-build --project ConformanceServer.csproj --urls {ServerUrl}", | |
| Arguments = $"run --project ConformanceServer.csproj --urls {ServerUrl}", |
| var outputBuilder = new StringBuilder(); | ||
| var errorBuilder = new StringBuilder(); | ||
|
|
||
| var process = new Process { StartInfo = startInfo }; |
There was a problem hiding this comment.
Disposable 'Process' is created but not disposed.
| if (result.Action == "accept" && result.Content != null) | ||
| { | ||
| return $"User responded: {result.Content["response"].GetString()}"; | ||
| } | ||
| else | ||
| { | ||
| return $"Elicitation {result.Action}"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement return - consider using '?' to express intent better.
| if (result.Action == "accept" && result.Content != null) | ||
| { | ||
| return $"Accepted with values: string={result.Content["stringField"].GetString()}, " + | ||
| $"number={result.Content["numberField"].GetInt32()}, " + | ||
| $"boolean={result.Content["booleanField"].GetBoolean()}"; | ||
| } | ||
| else | ||
| { | ||
| return $"Elicitation {result.Action}"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement return - consider using '?' to express intent better.
| if (result.Action == "accept" && result.Content != null) | ||
| { | ||
| return $"Accepted with values: color={result.Content["color"].GetString()}, " + | ||
| $"size={result.Content["size"].GetString()}"; | ||
| } | ||
| else | ||
| { | ||
| return $"Elicitation {result.Action}"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement return - consider using '?' to express intent better.
Motivation and Context
This PR adds a new sample server for use with the [MCP Conformance test suite].
How Has This Been Tested?
Tested with the server conformance test suite and all pass except one which I believe is a test error.
The failing scenario is tools-call-with-logging, which fails because it did not set the logging level at the start of the test.
Breaking Changes
No breaking changes.
Types of changes
Checklist
Additional context