diff --git a/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs b/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs index d740c7a4..b2eb1fc6 100644 --- a/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs +++ b/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs @@ -34,6 +34,11 @@ public enum McpServerTransport /// Configuration for a stdio MCP server. /// Communicates with the server over standard input/output. /// + // The base [JsonConverter] only applies when a contract is *declared* as McpServer, so a derived + // declared root (AcpJsonContext lists one per transport) would otherwise serialize raw properties + // with no "type" discriminator and no v1/v2 gate. Repeating the converter on each derived record + // closes that bypass: every root routes through the same polymorphic writer. + [JsonConverter(typeof(McpServerJsonConverter))] public sealed record StdioMcpServer : McpServer { /// @@ -85,6 +90,11 @@ public StdioMcpServer( /// Configuration for an HTTP MCP server. /// Communicates with the server over HTTP requests. /// + // The base [JsonConverter] only applies when a contract is *declared* as McpServer, so a derived + // declared root (AcpJsonContext lists one per transport) would otherwise serialize raw properties + // with no "type" discriminator and no v1/v2 gate. Repeating the converter on each derived record + // closes that bypass: every root routes through the same polymorphic writer. + [JsonConverter(typeof(McpServerJsonConverter))] public sealed record HttpMcpServer : McpServer { /// @@ -124,6 +134,11 @@ public HttpMcpServer(string name, string url, List? headers = nul /// Configuration for an SSE (Server-Sent Events) MCP server. /// Communicates with the server over an SSE stream. /// + // The base [JsonConverter] only applies when a contract is *declared* as McpServer, so a derived + // declared root (AcpJsonContext lists one per transport) would otherwise serialize raw properties + // with no "type" discriminator and no v1/v2 gate. Repeating the converter on each derived record + // closes that bypass: every root routes through the same polymorphic writer. + [JsonConverter(typeof(McpServerJsonConverter))] public sealed record SseMcpServer : McpServer { /// @@ -165,6 +180,11 @@ public SseMcpServer(string name, string url, List? headers = null /// stdio/http/sse): the spec requires a receiver to "preserve the raw payload" for a transport it does not /// recognize, leaving it to the Agent rather than the client to accept or reject it. /// + // The base [JsonConverter] only applies when a contract is *declared* as McpServer, so a derived + // declared root (AcpJsonContext lists one per transport) would otherwise serialize raw properties + // with no "type" discriminator and no v1/v2 gate. Repeating the converter on each derived record + // closes that bypass: every root routes through the same polymorphic writer. + [JsonConverter(typeof(McpServerJsonConverter))] public sealed record CustomMcpServer : McpServer { /// @@ -384,6 +404,11 @@ internal sealed class McpServerJsonConverter : JsonConverter internal const string SseV1OnlyMessage = "ACP MCP server transport sse is only available in protocolVersion 1."; + // The generated context validates a converter against its declared root with an exact-type + // CanConvert check; derived records reuse this converter for their own roots, so any type in + // the McpServer family is accepted and dispatch still happens on the runtime value in Write. + public override bool CanConvert(Type typeToConvert) => typeof(McpServer).IsAssignableFrom(typeToConvert); + public override McpServer? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { using var document = JsonDocument.ParseValue(ref reader); diff --git a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs index be0b819e..f06bbf49 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs @@ -187,6 +187,11 @@ namespace SalmonEgg.Acp.Serialization; [JsonSerializable(typeof(CustomMcpServer))] [JsonSerializable(typeof(McpHttpHeader))] [JsonSerializable(typeof(McpEnvVariable))] +// The derived McpServer records carry their own converter, so the generator no longer walks their +// properties and these two List<> infos must be declared explicitly: they are public members of the +// shipped context (AcpJsonContext.ListMcpHttpHeader / ListMcpEnvVariable). +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(List))] [JsonSerializable(typeof(ContentBlock))] [JsonSerializable(typeof(TextContentBlock))] [JsonSerializable(typeof(ImageContentBlock))] diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs index de7460df..141d1367 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs @@ -240,6 +240,26 @@ public void McpServerV2_TypedSse_RefusesToSerializeAtEveryRoot() Assert.Equal("events", stableSetup.RootElement.GetProperty("mcpServers")[0].GetProperty("name").GetString()); } + [Fact] + public void McpServerV2_DeclaredDerivedRoots_RouteThroughTheVersionGatedConverter() + { + // Arrange + var sse = new SseMcpServer("events", "https://example.test/events"); + var stdio = new StdioMcpServer { Name = "tools", Command = "tool" }; + + // Act + var gated = Assert.Throws(() => JsonSerializer.Serialize(sse, Wire.V2())); + using var v1Root = JsonDocument.Parse(JsonSerializer.Serialize(sse, Wire.V1())); + using var v2StdioRoot = JsonDocument.Parse(JsonSerializer.Serialize(stdio, Wire.V2())); + + // Assert + Assert.Contains(McpServerJsonConverter.SseV1OnlyMessage, gated.Message); + Assert.Equal("sse", v1Root.RootElement.GetProperty("type").GetString()); + // The discriminator must survive at every declared root too, or a derived-root payload + // could not round-trip through the v2 read path (which rejects a type-less object). + Assert.Equal("stdio", v2StdioRoot.RootElement.GetProperty("type").GetString()); + } + [Theory] [InlineData("")] [InlineData(",\"type\":null")]