From d3b508df55a565ed1504a05b5dfb165e382ca268 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Tue, 8 Sep 2026 18:42:06 +0000 Subject: [PATCH 1/6] fix(acp): complete isolated v2 wire contracts Apply negotiated identifiers and required message identities across direct, collection, and parent contracts. Preserve unknown command and resource metadata, honor explicit schema recovery rules, and keep v1-only state off draft wire. Keep live v2 disabled and preserve the published serialization surface. Validate 680 SDK tests, package compatibility against 1.0.0, and 38 draft consumer diagnostics; version-rule mutations fail 45 targeted cases. --- src/SalmonEgg.Acp/Content/ContentBlock.cs | 17 +- .../Content/ResourceLinkContentBlock.cs | 21 ++ .../Content/ResourceLinkDraftExtensions.cs | 65 ++++ src/SalmonEgg.Acp/Mcp/McpServerConfig.cs | 26 +- .../Protocol/ConfigOptionTypes.cs | 156 ++++++--- .../Protocol/IconJsonConverter.cs | 71 ++++ src/SalmonEgg.Acp/Protocol/InitializeTypes.cs | 59 ++-- .../Protocol/OtherSessionTypes.cs | 30 +- src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs | 47 +-- .../Protocol/SessionUpdateTypes.cs | 2 + .../Protocol/SlashCommandTypes.cs | 99 ++++++ .../Protocol/V2SupplementalTypes.cs | 7 + src/SalmonEgg.Acp/PublicSurface.Types.txt | 1 + .../Serialization/AcpJsonContext.cs | 3 + .../Serialization/AcpWireFormat.cs | 103 ++++++ .../DefaultableProtocolJsonConverters.cs | 86 +++++ .../Serialization/InitializeWireContract.cs | 38 ++ .../Protocol/AuthMethodTypesTests.cs | 2 +- .../Protocol/ConfigOptionV2Tests.cs | 2 +- .../SessionUpdateVersionSurfaceTests.cs | 6 +- .../Protocol/V2WireCommandInputTests.cs | 92 +++++ .../Protocol/V2WireContractTests.cs | 327 ++++++++++++++++++ .../Protocol/V2WireDefaultValueTests.cs | 223 ++++++++++++ .../Protocol/V2WireResourceLinkTests.cs | 116 +++++++ 24 files changed, 1482 insertions(+), 117 deletions(-) create mode 100644 src/SalmonEgg.Acp/Content/ResourceLinkDraftExtensions.cs create mode 100644 src/SalmonEgg.Acp/Protocol/IconJsonConverter.cs create mode 100644 src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs create mode 100644 src/SalmonEgg.Acp/Serialization/InitializeWireContract.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/V2WireCommandInputTests.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/V2WireDefaultValueTests.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/V2WireResourceLinkTests.cs diff --git a/src/SalmonEgg.Acp/Content/ContentBlock.cs b/src/SalmonEgg.Acp/Content/ContentBlock.cs index e6fe29ac5..2f659c191 100644 --- a/src/SalmonEgg.Acp/Content/ContentBlock.cs +++ b/src/SalmonEgg.Acp/Content/ContentBlock.cs @@ -3,6 +3,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; namespace SalmonEgg.Acp.Content { @@ -156,7 +157,7 @@ private static AudioContentBlock ReadAudio(JsonElement root) return block; } - private static ResourceLinkContentBlock ReadResourceLink(JsonElement root) + internal static ResourceLinkContentBlock ReadResourceLink(JsonElement root) { var block = new ResourceLinkContentBlock { @@ -166,6 +167,7 @@ private static ResourceLinkContentBlock ReadResourceLink(JsonElement root) Title = ReadString(root, "title"), Description = ReadString(root, "description"), Size = ReadInt64(root, "size"), + RawIcons = root.TryGetProperty("icons", out var icons) ? icons.Clone() : null, Annotations = ReadAnnotations(root), Meta = AcpMetaJson.Read(root) }; @@ -354,8 +356,13 @@ private static void WriteAudio(Utf8JsonWriter writer, AudioContentBlock value, J writer.WriteEndObject(); } - private static void WriteResourceLink(Utf8JsonWriter writer, ResourceLinkContentBlock value, JsonSerializerOptions options) + internal static void WriteResourceLink(Utf8JsonWriter writer, ResourceLinkContentBlock value, JsonSerializerOptions options) { + if (value.HasDraftIcons && AcpWireFormat.NegotiatedVersion(options) != AcpProtocolVersion.V2) + { + throw new JsonException("Authored resource icons require ACP v2 wire; received unknown fields remain passthrough."); + } + writer.WriteStartObject(); writer.WriteString("type", value.Type); WriteAnnotations(writer, value.Annotations, options); @@ -365,6 +372,12 @@ private static void WriteResourceLink(Utf8JsonWriter writer, ResourceLinkContent WriteNullableString(writer, "title", value.Title, options); WriteNullableString(writer, "description", value.Description, options); WriteNullableNumber(writer, "size", value.Size, options); + if (value.RawIcons is { } icons) + { + writer.WritePropertyName("icons"); + writer.WriteRawValue(icons.GetRawText()); + } + AcpMetaJson.Write(writer, value.Meta); writer.WriteEndObject(); } diff --git a/src/SalmonEgg.Acp/Content/ResourceLinkContentBlock.cs b/src/SalmonEgg.Acp/Content/ResourceLinkContentBlock.cs index cd2979e29..9b89ae6e6 100644 --- a/src/SalmonEgg.Acp/Content/ResourceLinkContentBlock.cs +++ b/src/SalmonEgg.Acp/Content/ResourceLinkContentBlock.cs @@ -1,3 +1,5 @@ +using System; +using System.Text.Json; using System.Text.Json.Serialization; namespace SalmonEgg.Acp.Content @@ -6,6 +8,7 @@ namespace SalmonEgg.Acp.Content /// Resource link content block. /// Represents a reference to an external resource (a URI link). /// + [JsonConverter(typeof(ResourceLinkContentBlockJsonConverter))] public sealed record ResourceLinkContentBlock : ContentBlock { /// @@ -52,6 +55,12 @@ public sealed record ResourceLinkContentBlock : ContentBlock [JsonPropertyName("size")] public long? Size { get; init; } + [JsonIgnore] + internal JsonElement? RawIcons { get; init; } + + [JsonIgnore] + internal bool HasDraftIcons { get; init; } + /// /// Creates a new resource link content block instance. /// @@ -84,4 +93,16 @@ public ResourceLinkContentBlock( Size = size; } } + + internal sealed class ResourceLinkContentBlockJsonConverter : JsonConverter + { + public override ResourceLinkContentBlock? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + return ContentBlockJsonConverter.ReadResourceLink(document.RootElement); + } + + public override void Write(Utf8JsonWriter writer, ResourceLinkContentBlock value, JsonSerializerOptions options) + => ContentBlockJsonConverter.WriteResourceLink(writer, value, options); + } } diff --git a/src/SalmonEgg.Acp/Content/ResourceLinkDraftExtensions.cs b/src/SalmonEgg.Acp/Content/ResourceLinkDraftExtensions.cs new file mode 100644 index 000000000..0ee5112f5 --- /dev/null +++ b/src/SalmonEgg.Acp/Content/ResourceLinkDraftExtensions.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Text.Json; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Content; + +/// Opt-in access to ACP v2 resource-link icon metadata. +[Experimental(AcpDraftProtocol.DiagnosticId, Message = AcpDraftProtocol.Message, UrlFormat = AcpDraftProtocol.UrlFormat)] +public static class ResourceLinkDraftExtensions +{ + /// Returns supported icon entries, applying the schema's default-on-error and skip-invalid-items rules. + public static IReadOnlyList GetIcons(this ResourceLinkContentBlock resource) + { + ArgumentNullException.ThrowIfNull(resource); + var result = new List(); + if (resource.RawIcons is not { ValueKind: JsonValueKind.Array } icons) + { + return result; + } + + var typeInfo = AcpWireFormat.For(AcpProtocolVersion.V2).TypeInfo(); + foreach (var item in icons.EnumerateArray()) + { + try + { + if (item.Deserialize(typeInfo) is { } icon) + { + result.Add(icon); + } + } + catch (JsonException) + { + // ResourceLink.icons alone grants per-item recovery; Icon.src remains required. + } + } + + return result; + } + + /// Creates a resource link with v2 icon metadata. Writing the result requires a v2 wire context. + public static ResourceLinkContentBlock WithIcons(this ResourceLinkContentBlock resource, IReadOnlyList icons) + { + ArgumentNullException.ThrowIfNull(resource); + ArgumentNullException.ThrowIfNull(icons); + var typeInfo = AcpWireFormat.For(AcpProtocolVersion.V2).TypeInfo(); + using var stream = new MemoryStream(); + using (var writer = new Utf8JsonWriter(stream)) + { + writer.WriteStartArray(); + foreach (var icon in icons) + { + JsonSerializer.Serialize(writer, icon, typeInfo); + } + + writer.WriteEndArray(); + } + + using var document = JsonDocument.Parse(stream.ToArray()); + return resource with { RawIcons = document.RootElement.Clone(), HasDraftIcons = true }; + } +} diff --git a/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs b/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs index cca603b28..477cf3f09 100644 --- a/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs +++ b/src/SalmonEgg.Acp/Mcp/McpServerConfig.cs @@ -385,7 +385,7 @@ internal sealed class McpServerJsonConverter : JsonConverter { using var document = JsonDocument.ParseValue(ref reader); var root = document.RootElement; - var transport = ResolveTransport(root); + var transport = ResolveTransport(root, options); return transport switch { @@ -429,22 +429,30 @@ public override void Write(Utf8JsonWriter writer, McpServer value, JsonSerialize } } - private static McpServerTransport ResolveTransport(JsonElement root) + private static McpServerTransport ResolveTransport(JsonElement root, JsonSerializerOptions options) { - if (!root.TryGetProperty("type", out var typeElement) - || typeElement.ValueKind != JsonValueKind.String) + var version = AcpWireFormat.NegotiatedVersion(options); + if (!root.TryGetProperty("type", out var typeElement)) { + if (version == AcpProtocolVersion.V2) + { + throw new JsonException("ACP v2 MCP server requires a string 'type' discriminator."); + } + return McpServerTransport.Stdio; } + if (typeElement.ValueKind != JsonValueKind.String) + { + throw new JsonException("MCP server 'type' must be a string when provided."); + } + return typeElement.GetString() switch { "stdio" => McpServerTransport.Stdio, "http" => McpServerTransport.Http, - "sse" => McpServerTransport.Sse, - // V2 schema "other" branch: any type value other than stdio/http/sse (including `_` extensions and - // future ACP variants) must preserve the raw payload for forward passthrough, leaving it to the Agent - // rather than the client to tighten. Read is purely tolerant and does not branch on version. + "sse" when version == AcpProtocolVersion.V1 => McpServerTransport.Sse, + // V2 removed SSE; it belongs to the same raw passthrough as future transports. _ => McpServerTransport.Custom }; } @@ -630,7 +638,7 @@ private static List ReadNameValueArray( private static void WriteStdio(Utf8JsonWriter writer, StdioMcpServer stdio, JsonSerializerOptions options) { writer.WriteStartObject(); - // The V2 schema discriminates stdio/http/sse via the `type` field; V1 stdio has no type field and is + // The V2 schema discriminates stdio/http via the `type` field; V1 stdio has no type field and is // identified implicitly by its absence. Write type only when the negotiated version is V2, so a V1 Agent // is never sent a field it does not recognize. if (AcpWireFormat.NegotiatedVersion(options) >= AcpProtocolVersion.V2) diff --git a/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs b/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs index f8ac0e769..4e9fbdadd 100644 --- a/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs @@ -38,6 +38,8 @@ public sealed record ConfigOption : AcpProtocolObject [JsonIgnore] public List OptionGroups { get; init; } = new(); + + internal JsonElement? RawPayload { get; init; } } public sealed record ConfigOptionValue : AcpProtocolObject @@ -52,6 +54,7 @@ public sealed record ConfigOptionValue : AcpProtocolObject public string? Description { get; init; } } +[JsonConverter(typeof(ConfigOptionGroupJsonConverter))] public sealed record ConfigOptionGroup : AcpProtocolObject { [JsonPropertyName("group")] @@ -89,7 +92,7 @@ internal sealed class ConfigOptionJsonConverter : JsonConverter if (string.Equals(type, "select", System.StringComparison.Ordinal)) { currentValueText = ReadRequiredString(root, "currentValue"); - ReadSelectOptions(root, selectOptions, optionGroups); + ReadSelectOptions(root, selectOptions, optionGroups, options); } else if (string.Equals(type, "boolean", System.StringComparison.Ordinal)) { @@ -104,9 +107,7 @@ internal sealed class ConfigOptionJsonConverter : JsonConverter return new ConfigOption { - // v1 calls this id; v2 renamed it configId. Reading accepts both versions, while writing - // branches on the negotiated context so neither peer sees the other's field. - Id = ReadRequiredString(root, "configId", "id"), + Id = ReadRequiredString(root, IdPropertyName(options)), Name = ReadRequiredString(root, "name"), Description = ReadOptionalString(root, "description"), Category = ReadOptionalString(root, "category"), @@ -115,16 +116,21 @@ internal sealed class ConfigOptionJsonConverter : JsonConverter CurrentBooleanValue = currentBoolean, Options = selectOptions, OptionGroups = optionGroups, - Meta = AcpMetaJson.Read(root) + Meta = AcpMetaJson.Read(root), + RawPayload = root.Clone() }; } public override void Write(Utf8JsonWriter writer, ConfigOption value, JsonSerializerOptions options) { + if (value.Type is not "select" and not "boolean" && value.RawPayload is { } rawPayload) + { + writer.WriteRawValue(rawPayload.GetRawText()); + return; + } + writer.WriteStartObject(); - writer.WriteString( - AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2 ? "configId" : "id", - value.Id); + writer.WriteString(IdPropertyName(options), value.Id); writer.WriteString("name", value.Name); WriteOptionalString(writer, "description", value.Description, options); WriteOptionalString(writer, "category", value.Category, options); @@ -163,13 +169,61 @@ public override void Write(Utf8JsonWriter writer, ConfigOption value, JsonSerial } AcpMetaJson.Write(writer, value.Meta); + WriteUnknownFields(writer, value, options); + writer.WriteEndObject(); + } + + internal static string GroupPropertyName(JsonSerializerOptions options) + => AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2 ? "groupId" : "group"; + + internal static ConfigOptionGroup ReadGroup(JsonElement element, JsonSerializerOptions options) + { + if (element.ValueKind != JsonValueKind.Object || !element.TryGetProperty("options", out var optionsElement)) + { + throw new JsonException("ACP session config option group requires an options array."); + } + + var groupOptions = new List(); + if (optionsElement.ValueKind == JsonValueKind.Array) + { + foreach (var item in optionsElement.EnumerateArray()) + { + try + { + groupOptions.Add(ReadOption(item)); + } + catch (JsonException) + { + // Both schemas mark group.options as default-on-error and skip-invalid-items. + } + } + } + + return new ConfigOptionGroup + { + Group = ReadRequiredString(element, GroupPropertyName(options)), + Name = ReadRequiredString(element, "name"), + Options = groupOptions, + Meta = AcpMetaJson.Read(element) + }; + } + + internal static void WriteGroup(Utf8JsonWriter writer, ConfigOptionGroup group, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString(GroupPropertyName(options), group.Group); + writer.WriteString("name", group.Name); + writer.WritePropertyName("options"); + WriteOptions(writer, group.Options, options); + AcpMetaJson.Write(writer, group.Meta); writer.WriteEndObject(); } private static void ReadSelectOptions( JsonElement root, List options, - List optionGroups) + List optionGroups, + JsonSerializerOptions serializerOptions) { if (!root.TryGetProperty("options", out var optionsElement) || optionsElement.ValueKind != JsonValueKind.Array) @@ -185,7 +239,7 @@ private static void ReadSelectOptions( throw new JsonException("ACP select option entries must be objects."); } - var isGroup = item.TryGetProperty("group", out _); + var isGroup = item.TryGetProperty(GroupPropertyName(serializerOptions), out _); if (grouped.HasValue && grouped.Value != isGroup) { throw new JsonException("ACP select config options cannot mix grouped and ungrouped values."); @@ -194,7 +248,7 @@ private static void ReadSelectOptions( grouped = isGroup; if (isGroup) { - optionGroups.Add(ReadGroup(item)); + optionGroups.Add(ReadGroup(item, serializerOptions)); } else { @@ -203,29 +257,6 @@ private static void ReadSelectOptions( } } - private static ConfigOptionGroup ReadGroup(JsonElement element) - { - if (!element.TryGetProperty("options", out var optionsElement) - || optionsElement.ValueKind != JsonValueKind.Array) - { - throw new JsonException("ACP session config option group requires an options array."); - } - - var groupOptions = new List(); - foreach (var item in optionsElement.EnumerateArray()) - { - groupOptions.Add(ReadOption(item)); - } - - return new ConfigOptionGroup - { - Group = ReadRequiredString(element, "group"), - Name = ReadRequiredString(element, "name"), - Options = groupOptions, - Meta = AcpMetaJson.Read(element) - }; - } - private static ConfigOptionValue ReadOption(JsonElement element) => new() { @@ -235,18 +266,18 @@ private static ConfigOptionValue ReadOption(JsonElement element) Meta = AcpMetaJson.Read(element) }; - private static string ReadRequiredString(JsonElement root, params string[] propertyNames) + private static string IdPropertyName(JsonSerializerOptions options) + => AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2 ? "configId" : "id"; + + private static string ReadRequiredString(JsonElement root, string propertyName) { - foreach (var propertyName in propertyNames) + if (root.ValueKind == JsonValueKind.Object + && root.TryGetProperty(propertyName, out var property) && property.ValueKind == JsonValueKind.String) { - if (root.TryGetProperty(propertyName, out var property) - && property.ValueKind == JsonValueKind.String) - { - return property.GetString() ?? string.Empty; - } + return property.GetString()!; } - throw new JsonException($"ACP session config option requires string property '{propertyNames[0]}'."); + throw new JsonException($"ACP session config option requires string property '{propertyName}'."); } private static string? ReadOptionalString(JsonElement root, string propertyName) @@ -258,7 +289,26 @@ private static string ReadRequiredString(JsonElement root, params string[] prope return property.ValueKind == JsonValueKind.String ? property.GetString() - : throw new JsonException($"ACP session config option property '{propertyName}' must be a string or null."); + : null; + } + + private static void WriteUnknownFields(Utf8JsonWriter writer, ConfigOption value, JsonSerializerOptions options) + { + if (value.RawPayload is not { } payload) + { + return; + } + + var idProperty = IdPropertyName(options); + foreach (var property in payload.EnumerateObject()) + { + if (property.Name != idProperty && property.Name is not "name" and not "description" and not "category" + and not "type" and not "currentValue" and not "options" and not "_meta") + { + writer.WritePropertyName(property.Name); + writer.WriteRawValue(property.Value.GetRawText()); + } + } } private static void WriteGroups( @@ -269,13 +319,7 @@ private static void WriteGroups( writer.WriteStartArray(); foreach (var group in groups) { - writer.WriteStartObject(); - writer.WriteString("group", group.Group); - writer.WriteString("name", group.Name); - writer.WritePropertyName("options"); - WriteOptions(writer, group.Options, serializerOptions); - AcpMetaJson.Write(writer, group.Meta); - writer.WriteEndObject(); + WriteGroup(writer, group, serializerOptions); } writer.WriteEndArray(); @@ -317,3 +361,15 @@ private static void WriteOptionalString( } } } + +internal sealed class ConfigOptionGroupJsonConverter : JsonConverter +{ + public override ConfigOptionGroup? Read(ref Utf8JsonReader reader, System.Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + return ConfigOptionJsonConverter.ReadGroup(document.RootElement, options); + } + + public override void Write(Utf8JsonWriter writer, ConfigOptionGroup value, JsonSerializerOptions options) + => ConfigOptionJsonConverter.WriteGroup(writer, value, options); +} diff --git a/src/SalmonEgg.Acp/Protocol/IconJsonConverter.cs b/src/SalmonEgg.Acp/Protocol/IconJsonConverter.cs new file mode 100644 index 000000000..13a36eac7 --- /dev/null +++ b/src/SalmonEgg.Acp/Protocol/IconJsonConverter.cs @@ -0,0 +1,71 @@ +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace SalmonEgg.Acp.Protocol; + +internal sealed class IconJsonConverter : JsonConverter +{ + public override Icon? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + var root = document.RootElement; + if (root.ValueKind != JsonValueKind.Object + || !root.TryGetProperty("src", out var source) || source.ValueKind != JsonValueKind.String) + { + throw new JsonException("Icon requires string 'src'."); + } + + List? sizes = null; + if (root.TryGetProperty("sizes", out var rawSizes) && rawSizes.ValueKind == JsonValueKind.Array) + { + sizes = new List(); + foreach (var size in rawSizes.EnumerateArray()) + { + if (size.ValueKind == JsonValueKind.String) sizes.Add(size.GetString()!); + } + } + + return new Icon + { + Src = source.GetString()!, + MimeType = ReadOptionalString(root, "mimeType"), + Theme = ReadOptionalString(root, "theme"), + Sizes = sizes, + RawPayload = root.Clone() + }; + } + + public override void Write(Utf8JsonWriter writer, Icon value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString("src", value.Src); + if (value.MimeType is not null) writer.WriteString("mimeType", value.MimeType); + if (value.Theme is not null) writer.WriteString("theme", value.Theme); + if (value.Sizes is not null) + { + writer.WritePropertyName("sizes"); + writer.WriteStartArray(); + foreach (var size in value.Sizes) writer.WriteStringValue(size); + writer.WriteEndArray(); + } + + if (value.RawPayload is { } root) + { + foreach (var property in root.EnumerateObject()) + { + if (property.Name is not "src" and not "mimeType" and not "sizes" and not "theme") + { + writer.WritePropertyName(property.Name); + writer.WriteRawValue(property.Value.GetRawText()); + } + } + } + + writer.WriteEndObject(); + } + + private static string? ReadOptionalString(JsonElement root, string name) + => root.TryGetProperty(name, out var property) && property.ValueKind == JsonValueKind.String ? property.GetString() : null; +} diff --git a/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs b/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs index fbe5a1ec8..bcc411bc7 100644 --- a/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs @@ -765,12 +765,13 @@ internal sealed class InitializeParamsJsonConverter : JsonConverter)options.GetTypeInfo(typeof(ClientInfo))); + JsonSerializer.Serialize(writer, value.ClientInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(ClientInfo))); writer.WritePropertyName("clientCapabilities"); JsonSerializer.Serialize(writer, value.ClientCapabilities, (JsonTypeInfo)options.GetTypeInfo(typeof(ClientCapabilities))); } else { writer.WritePropertyName("info"); - JsonSerializer.Serialize(writer, value.ClientInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(ClientInfo))); + JsonSerializer.Serialize(writer, value.ClientInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(ClientInfo))); WriteClientCapabilitiesV2(writer, value.ClientCapabilities, options); } @@ -812,10 +817,11 @@ private static int ReadProtocolVersion(JsonElement root) return version.GetInt32(); } - private static ClientInfo ReadClientInfo(JsonElement root, JsonSerializerOptions options) + private static ClientInfo ReadClientInfo(JsonElement root, int protocolVersion, JsonSerializerOptions options) { - if (root.TryGetProperty("info", out var info)) + if (protocolVersion == AcpProtocolVersion.V2) { + var info = InitializeWireContract.RequireInfo(root); return JsonSerializer.Deserialize(info.GetRawText(), (JsonTypeInfo)options.GetTypeInfo(typeof(ClientInfo))) ?? new ClientInfo(); } @@ -827,11 +833,16 @@ private static ClientInfo ReadClientInfo(JsonElement root, JsonSerializerOptions return new ClientInfo(); } - private static ClientCapabilities ReadClientCapabilities(JsonElement root, JsonSerializerOptions options) + private static ClientCapabilities ReadClientCapabilities(JsonElement root, int protocolVersion, JsonSerializerOptions options) { - if (root.TryGetProperty("capabilities", out var capabilities)) + if (protocolVersion == AcpProtocolVersion.V2) { - return JsonSerializer.Deserialize(capabilities.GetRawText(), (JsonTypeInfo)options.GetTypeInfo(typeof(ClientCapabilities))) ?? new ClientCapabilities(); + if (!root.TryGetProperty("capabilities", out var capabilities) || capabilities.ValueKind != JsonValueKind.Object) + { + return new ClientCapabilities(); + } + + return JsonSerializer.Deserialize(capabilities.GetRawText(), AcpWireFormat.For(protocolVersion).TypeInfo()) ?? new ClientCapabilities(); } if (root.TryGetProperty("clientCapabilities", out var clientCapabilities)) @@ -872,8 +883,8 @@ internal sealed class InitializeResponseJsonConverter : JsonConverter)options.GetTypeInfo(typeof(AgentInfo))); + JsonSerializer.Serialize(writer, value.AgentInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(AgentInfo))); writer.WritePropertyName("agentCapabilities"); JsonSerializer.Serialize(writer, value.AgentCapabilities, (JsonTypeInfo)options.GetTypeInfo(typeof(AgentCapabilities))); } else { writer.WritePropertyName("info"); - JsonSerializer.Serialize(writer, value.AgentInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(AgentInfo))); - WriteAgentCapabilitiesV2(writer, value.AgentCapabilities, options); + JsonSerializer.Serialize(writer, value.AgentInfo, (JsonTypeInfo)options.GetTypeInfo(typeof(AgentInfo))); + WriteAgentCapabilitiesV2(writer, value.AgentCapabilities, AcpWireFormat.For(AcpProtocolVersion.V2).Options); } writer.WritePropertyName("authMethods"); @@ -916,10 +932,11 @@ private static int ReadProtocolVersion(JsonElement root) return version.GetInt32(); } - private static AgentInfo ReadAgentInfo(JsonElement root, JsonSerializerOptions options) + private static AgentInfo ReadAgentInfo(JsonElement root, int protocolVersion, JsonSerializerOptions options) { - if (root.TryGetProperty("info", out var info)) + if (protocolVersion == AcpProtocolVersion.V2) { + var info = InitializeWireContract.RequireInfo(root); return JsonSerializer.Deserialize(info.GetRawText(), (JsonTypeInfo)options.GetTypeInfo(typeof(AgentInfo))) ?? new AgentInfo(); } @@ -931,11 +948,13 @@ private static AgentInfo ReadAgentInfo(JsonElement root, JsonSerializerOptions o return new AgentInfo(); } - private static AgentCapabilities ReadAgentCapabilities(JsonElement root, JsonSerializerOptions options) + private static AgentCapabilities ReadAgentCapabilities(JsonElement root, int protocolVersion, JsonSerializerOptions options) { - if (root.TryGetProperty("capabilities", out var capabilities)) + if (protocolVersion == AcpProtocolVersion.V2) { - return ReadAgentCapabilitiesV2(capabilities, options); + return root.TryGetProperty("capabilities", out var capabilities) && capabilities.ValueKind == JsonValueKind.Object + ? ReadAgentCapabilitiesV2(capabilities, AcpWireFormat.For(protocolVersion).Options) + : new AgentCapabilities(); } if (root.TryGetProperty("agentCapabilities", out var agentCapabilities)) diff --git a/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs b/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs index e32d26b2a..368cd7c4f 100644 --- a/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs @@ -376,17 +376,11 @@ public SessionResumeParams( public sealed record SessionResumeResponse : AcpProtocolObject { /// - /// Session mode state (optional; the standard ACP form is a SessionModeState object). + /// A static instance representing resume completion. /// - [JsonPropertyName("modes")] - [JsonConverter(typeof(SessionModesStateJsonConverter))] - public SessionModesState? Modes { get; init; } + public static readonly SessionResumeResponse Completed = new SessionResumeResponse(); - /// - /// List of available configuration options (optional). - /// - [JsonPropertyName("configOptions")] - public List? ConfigOptions { get; init; } + private List? _configOptions; /// /// Creates a new SessionResumeResponse instance. @@ -407,9 +401,23 @@ public SessionResumeResponse(SessionModesState? modes, List? confi } /// - /// A static instance representing resume completion. + /// Session mode state (optional; the standard ACP form is a SessionModeState object). /// - public static readonly SessionResumeResponse Completed = new SessionResumeResponse(); + [JsonPropertyName("modes")] + [JsonConverter(typeof(SessionModesStateJsonConverter))] + public SessionModesState? Modes { get; init; } + + /// + /// List of available configuration options. V1 can omit the snapshot; V2 defaults it to an empty list. + /// + [JsonPropertyName("configOptions")] + public List? ConfigOptions + { + get => _configOptions; + init => _configOptions = value; + } + + internal void SetDefaultConfigOptions() => _configOptions ??= new List(); } /// diff --git a/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs b/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs index a93aef8d2..01d04a7cc 100644 --- a/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs @@ -61,26 +61,7 @@ public SessionNewParams( /// public sealed record SessionNewResponse : AcpProtocolObject { - /// - /// The ID of the newly created session. - /// - [JsonPropertyName("sessionId")] - public string SessionId { get; init; } = string.Empty; - - /// - /// Session mode state (optional; the ACP standard shape is a SessionModeState object). - /// - [JsonPropertyName("modes")] - [JsonConverter(typeof(SessionModesStateJsonConverter))] - public SessionModesState? Modes { get; init; } - - - /// - /// List of available configuration options (optional). - /// - [JsonPropertyName("configOptions")] - public List? ConfigOptions { get; init; } - + private List? _configOptions; /// /// Creates a new SessionNewResponse instance. @@ -101,6 +82,32 @@ public SessionNewResponse(string sessionId, SessionModesState? modes = null, Lis Modes = modes; ConfigOptions = configOptions; } + + /// + /// The ID of the newly created session. + /// + [JsonPropertyName("sessionId")] + public string SessionId { get; init; } = string.Empty; + + /// + /// Session mode state (optional; the ACP standard shape is a SessionModeState object). + /// + [JsonPropertyName("modes")] + [JsonConverter(typeof(SessionModesStateJsonConverter))] + public SessionModesState? Modes { get; init; } + + + /// + /// List of available configuration options. V1 can omit the snapshot; V2 defaults it to an empty list. + /// + [JsonPropertyName("configOptions")] + public List? ConfigOptions + { + get => _configOptions; + init => _configOptions = value; + } + + internal void SetDefaultConfigOptions() => _configOptions ??= new List(); } /// diff --git a/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs b/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs index ed7839845..b213857b7 100644 --- a/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs @@ -634,6 +634,8 @@ public sealed record AgentWholeThoughtUpdate : WholeMessageUpdate public abstract record ContentChunkUpdate : SessionUpdate { [JsonPropertyName("messageId")] + // The unversioned generated contract is v1; the v2 resolver replaces this defaultable reader. + [JsonConverter(typeof(DefaultableStringJsonConverter))] public string? MessageId { get; init; } } diff --git a/src/SalmonEgg.Acp/Protocol/SlashCommandTypes.cs b/src/SalmonEgg.Acp/Protocol/SlashCommandTypes.cs index 698225640..45c00e76b 100644 --- a/src/SalmonEgg.Acp/Protocol/SlashCommandTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/SlashCommandTypes.cs @@ -1,5 +1,9 @@ +using System; using System.Collections.Generic; +using System.Text.Json; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; +using SalmonEgg.Acp.Serialization; namespace SalmonEgg.Acp.Protocol; @@ -22,11 +26,106 @@ public sealed record AvailableCommand : AcpProtocolObject public string Description { get; init; } = string.Empty; [JsonPropertyName("input")] + [JsonConverter(typeof(DefaultableAvailableCommandInputJsonConverter))] public AvailableCommandInput? Input { get; init; } } +[JsonConverter(typeof(AvailableCommandInputJsonConverter))] public sealed record AvailableCommandInput : AcpProtocolObject { [JsonPropertyName("hint")] public string Hint { get; init; } = string.Empty; + + internal JsonElement? RawPayload { get; init; } +} + +internal sealed class AvailableCommandInputJsonConverter : JsonConverter +{ + public override AvailableCommandInput? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + var root = document.RootElement; + if (root.ValueKind != JsonValueKind.Object) + { + throw new JsonException("Command input must be an object."); + } + + if (AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2) + { + if (!root.TryGetProperty("type", out var type)) + { + throw new JsonException("ACP v2 command input requires 'type'."); + } + + if (type.ValueKind != JsonValueKind.String) + { + throw new JsonException("Command input 'type' must be a string."); + } + + if (type.GetString() != "text") + { + return new AvailableCommandInput { RawPayload = root.Clone() }; + } + } + + if (!root.TryGetProperty("hint", out var hint) || hint.ValueKind != JsonValueKind.String) + { + throw new JsonException("Text command input requires string 'hint'."); + } + + return new AvailableCommandInput { Hint = hint.GetString()!, Meta = AcpMetaJson.Read(root), RawPayload = root.Clone() }; + } + + public override void Write(Utf8JsonWriter writer, AvailableCommandInput value, JsonSerializerOptions options) + { + var isV2 = AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2; + if (isV2 && value.RawPayload is { } raw && raw.TryGetProperty("type", out var type) + && type.ValueKind == JsonValueKind.String && type.GetString() != "text") + { + writer.WriteRawValue(raw.GetRawText()); + return; + } + + writer.WriteStartObject(); + if (isV2) + { + writer.WriteString("type", "text"); + } + + writer.WriteString("hint", value.Hint); + AcpMetaJson.Write(writer, value.Meta); + if (value.RawPayload is { } payload) + { + foreach (var property in payload.EnumerateObject()) + { + if (property.Name is not "hint" and not "_meta" && (!isV2 || property.Name != "type")) + { + writer.WritePropertyName(property.Name); + writer.WriteRawValue(property.Value.GetRawText()); + } + } + } + + writer.WriteEndObject(); + } +} + +internal sealed class DefaultableAvailableCommandInputJsonConverter : JsonConverter +{ + public override AvailableCommandInput? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + try + { + return document.RootElement.Deserialize((JsonTypeInfo)options.GetTypeInfo(typeof(AvailableCommandInput))); + } + catch (JsonException) + { + // AvailableCommand.input explicitly permits default-on-error; the union root does not. + return null; + } + } + + public override void Write(Utf8JsonWriter writer, AvailableCommandInput value, JsonSerializerOptions options) + => JsonSerializer.Serialize(writer, value, (JsonTypeInfo)options.GetTypeInfo(typeof(AvailableCommandInput))); } diff --git a/src/SalmonEgg.Acp/Protocol/V2SupplementalTypes.cs b/src/SalmonEgg.Acp/Protocol/V2SupplementalTypes.cs index 90403324d..35168a685 100644 --- a/src/SalmonEgg.Acp/Protocol/V2SupplementalTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/V2SupplementalTypes.cs @@ -41,6 +41,7 @@ public static class IconThemeKind /// An icon supplied by an Agent in v2 metadata. [Experimental(AcpDraftProtocol.DiagnosticId, Message = AcpDraftProtocol.Message, UrlFormat = AcpDraftProtocol.UrlFormat)] +[JsonConverter(typeof(IconJsonConverter))] public sealed record Icon { /// URI of the icon resource. Required. @@ -58,12 +59,18 @@ public sealed record Icon /// Preferred display theme, when any. [JsonPropertyName("theme")] public string? Theme { get; init; } + + internal JsonElement? RawPayload { get; init; } } /// V2 command input specification for free text after the command name. [Experimental(AcpDraftProtocol.DiagnosticId, Message = AcpDraftProtocol.Message, UrlFormat = AcpDraftProtocol.UrlFormat)] public sealed record TextCommandInput : AcpProtocolObject { + /// ACP v2 text-input discriminator. + [JsonPropertyName("type")] + public string Type => "text"; + /// Input hint shown to the user. Required. [JsonPropertyName("hint")] public string Hint { get; init; } = string.Empty; diff --git a/src/SalmonEgg.Acp/PublicSurface.Types.txt b/src/SalmonEgg.Acp/PublicSurface.Types.txt index 17903a80e..6727373ca 100644 --- a/src/SalmonEgg.Acp/PublicSurface.Types.txt +++ b/src/SalmonEgg.Acp/PublicSurface.Types.txt @@ -39,6 +39,7 @@ SalmonEgg.Acp.Content.EmbeddedResource stable SalmonEgg.Acp.Content.ImageContentBlock stable SalmonEgg.Acp.Content.ResourceContentBlock stable SalmonEgg.Acp.Content.ResourceLinkContentBlock stable +SalmonEgg.Acp.Content.ResourceLinkDraftExtensions draft SalmonEgg.Acp.Content.TextContentBlock stable SalmonEgg.Acp.JsonRpc.AcpException stable SalmonEgg.Acp.JsonRpc.AcpFrame stable diff --git a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs index 3933e4648..c4ecef0b7 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs @@ -163,6 +163,9 @@ namespace SalmonEgg.Acp.Serialization; [JsonSerializable(typeof(ConfigOption))] [JsonSerializable(typeof(ConfigOptionValue))] [JsonSerializable(typeof(ConfigOptionGroup))] +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(List))] [JsonSerializable(typeof(TerminalCreateRequest))] [JsonSerializable(typeof(TerminalCreateResponse))] [JsonSerializable(typeof(TerminalOutputRequest))] diff --git a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs index 356c1f37b..faaa7793e 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs @@ -103,9 +103,112 @@ internal static int NegotiatedVersion(JsonSerializerOptions options) => ApplyNegotiatedSurface(info.PolymorphismOptions); } + if (info?.Kind == JsonTypeInfoKind.Object) + { + ApplyObjectContract(info); + } + return info; } + private void ApplyObjectContract(JsonTypeInfo info) + { + if (Version == AcpProtocolVersion.V2 + && (typeof(ContentChunkUpdate).IsAssignableFrom(info.Type) || typeof(WholeMessageUpdate).IsAssignableFrom(info.Type))) + { + var messageId = FindProperty(info, "messageId"); + messageId.IsRequired = true; + messageId.CustomConverter = null; + info.OnDeserialized = static value => RequireMessageId(value); + info.OnSerializing = static value => RequireMessageId(value); + } + + if (Version != AcpProtocolVersion.V2) + { + return; + } + + if (info.Type == typeof(SessionNewResponse) || info.Type == typeof(SessionResumeResponse)) + { + IgnoreProperty(info, "modes", new IgnoredProtocolPropertyJsonConverter()); + FindProperty(info, "configOptions").CustomConverter = new DefaultableConfigOptionsJsonConverter(); + info.OnDeserialized = static value => NormalizeConfigOptions(value); + } + else if (info.Type == typeof(ConfigOptionUpdate) || info.Type == typeof(SessionSetConfigOptionResponse)) + { + var configOptions = FindProperty(info, "configOptions"); + configOptions.IsRequired = true; + configOptions.CustomConverter = new DefaultableConfigOptionsJsonConverter(); + info.OnSerializing = static value => RequireConfigOptions(value); + } + else if (info.Type == typeof(AgentAuthCapabilities)) + { + IgnoreProperty(info, "logout", new IgnoredProtocolPropertyJsonConverter()); + } + else if (info.Type == typeof(ClientCapabilities)) + { + info.OnSerializing = static value => InitializeClientProtocolPolicy.Validate(AcpProtocolVersion.V2, (ClientCapabilities)value); + IgnoreProperty(info, "fs", new IgnoredProtocolPropertyJsonConverter()); + IgnoreProperty(info, "terminal", new IgnoredProtocolPropertyJsonConverter()); + IgnoreProperty(info, "session", new IgnoredProtocolPropertyJsonConverter()); + } + } + + private static void RequireMessageId(object value) + { + var id = value is ContentChunkUpdate chunk ? chunk.MessageId : ((WholeMessageUpdate)value).MessageId; + if (id is null) + { + throw new JsonException("ACP v2 message update requires string 'messageId'."); + } + } + + private static void NormalizeConfigOptions(object value) + { + // The v2 schema uses a defaultable array here, rather than v1's nullable snapshot. + if (value is SessionNewResponse created && created.ConfigOptions is null) + { + created.SetDefaultConfigOptions(); + } + else if (value is SessionResumeResponse resumed && resumed.ConfigOptions is null) + { + resumed.SetDefaultConfigOptions(); + } + } + + private static void RequireConfigOptions(object value) + { + var options = value is ConfigOptionUpdate update + ? update.ConfigOptions + : ((SessionSetConfigOptionResponse)value).ConfigOptions; + if (options is null) + { + throw new JsonException("ACP v2 configuration update requires 'configOptions'."); + } + } + + private static JsonPropertyInfo FindProperty(JsonTypeInfo info, string name) + { + foreach (var property in info.Properties) + { + if (property.Name == name) + { + return property; + } + } + + throw new InvalidOperationException($"The {info.Type.Name} contract has no '{name}' property."); + } + + private static void IgnoreProperty(JsonTypeInfo info, string name, JsonConverter converter) + { + // Source-generated record constructors bind parameters to these properties. Retain that + // metadata while removing the older wire behavior, rather than breaking constructor binding. + var property = FindProperty(info, name); + property.CustomConverter = converter; + property.ShouldSerialize = static (_, _) => false; + } + /// /// Replaces the polymorphic registrations with the ones the negotiated version defines. /// diff --git a/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs new file mode 100644 index 000000000..a43af8316 --- /dev/null +++ b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs @@ -0,0 +1,86 @@ +using System; +using System.Collections.Generic; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; +using SalmonEgg.Acp.Protocol; + +namespace SalmonEgg.Acp.Serialization; + +internal sealed class IgnoredProtocolPropertyJsonConverter : JsonConverter +{ + public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + reader.Skip(); + return default; + } + + public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options) + => writer.WriteNullValue(); +} + +internal sealed class DefaultableStringJsonConverter : JsonConverter +{ + public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.String) + { + return reader.GetString(); + } + + reader.Skip(); + return null; + } + + public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) + => writer.WriteStringValue(value); +} + +internal sealed class DefaultableConfigOptionsJsonConverter : JsonConverter> +{ + public override bool HandleNull => true; + + public override List Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + var result = new List(); + if (document.RootElement.ValueKind != JsonValueKind.Array) + { + return result; + } + + var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ConfigOption)); + foreach (var item in document.RootElement.EnumerateArray()) + { + try + { + if (item.Deserialize(typeInfo) is { } option) + { + result.Add(option); + } + } + catch (JsonException) + { + // Only configOptions has this pair of explicit schema recovery annotations: + // x-deserialize-default-on-error and x-deserialize-skip-invalid-items. + } + } + + return result; + } + + public override void Write(Utf8JsonWriter writer, List value, JsonSerializerOptions options) + { + writer.WriteStartArray(); + if (value is not null) + { + var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ConfigOption)); + foreach (var option in value) + { + JsonSerializer.Serialize(writer, option, typeInfo); + } + } + + writer.WriteEndArray(); + } +} diff --git a/src/SalmonEgg.Acp/Serialization/InitializeWireContract.cs b/src/SalmonEgg.Acp/Serialization/InitializeWireContract.cs new file mode 100644 index 000000000..f5ff47a74 --- /dev/null +++ b/src/SalmonEgg.Acp/Serialization/InitializeWireContract.cs @@ -0,0 +1,38 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; + +namespace SalmonEgg.Acp.Serialization; + +internal static class InitializeWireContract +{ + private const string InvalidInfoMessage = "ACP v2 initialize requires 'info' with string 'name' and 'version'."; + + internal static JsonElement RequireInfo(JsonElement root) + { + // v2's Implementation is required; capabilities is separately defaultable by schema. + // Keeping these checks separate prevents a malformed identity being treated as an empty peer. + if (!root.TryGetProperty("info", out var info) + || info.ValueKind != JsonValueKind.Object + || !info.TryGetProperty("name", out var name) + || name.ValueKind != JsonValueKind.String + || !info.TryGetProperty("version", out var version) + || version.ValueKind != JsonValueKind.String) + { + throw new JsonException(InvalidInfoMessage); + } + + return info; + } + + internal static void RequireInfo(ClientInfo? info) => RequireInfo(info?.Name, info?.Version); + + internal static void RequireInfo(AgentInfo? info) => RequireInfo(info?.Name, info?.Version); + + private static void RequireInfo(string? name, string? version) + { + if (name is null || version is null) + { + throw new JsonException(InvalidInfoMessage); + } + } +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs index ff907bf24..245b71431 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs @@ -92,7 +92,7 @@ public void SerializeInitializeResponse_UnsupportedMethods_PreservesDiscriminato {"{{{idProperty}}}":"login","name":"Login","type":" ","args":["login"],"env":{"AUTH_MODE":"interactive"},"future":{"key":1}} """; var initializeJson = $$""" - {"protocolVersion":{{version}},"agentCapabilities":{},"capabilities":{},"authMethods":[{{methodJson}}]} + {"protocolVersion":{{version}},"info":{"name":"agent","version":"1"},"agentCapabilities":{},"capabilities":{},"authMethods":[{{methodJson}}]} """; var response = Assert.IsType( JsonSerializer.Deserialize(initializeJson, AcpJsonContext.Default.InitializeResponse)); diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/ConfigOptionV2Tests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/ConfigOptionV2Tests.cs index 71c9d8615..d50c06c35 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/ConfigOptionV2Tests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/ConfigOptionV2Tests.cs @@ -12,7 +12,7 @@ public void ConfigOption_ReadsV2ConfigId() { var option = JsonSerializer.Deserialize( "{\"configId\":\"mode\",\"name\":\"Mode\",\"type\":\"boolean\",\"currentValue\":true}", - AcpJsonContext.Default.ConfigOption); + Wire.V2()); Assert.Equal("mode", option!.Id); } diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/SessionUpdateVersionSurfaceTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/SessionUpdateVersionSurfaceTests.cs index 6e76f528d..542b7730e 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/SessionUpdateVersionSurfaceTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/SessionUpdateVersionSurfaceTests.cs @@ -48,9 +48,9 @@ private static readonly (string Discriminator, string UpdateJson)[] s_v1Only = private static readonly (string Discriminator, string UpdateJson)[] s_shared = [ - ("agent_message_chunk", """{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"hi"}}"""), - ("user_message_chunk", """{"sessionUpdate":"user_message_chunk","content":{"type":"text","text":"ask"}}"""), - ("agent_thought_chunk", """{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"think"}}"""), + ("agent_message_chunk", """{"sessionUpdate":"agent_message_chunk","messageId":"m-1","content":{"type":"text","text":"hi"}}"""), + ("user_message_chunk", """{"sessionUpdate":"user_message_chunk","messageId":"m-2","content":{"type":"text","text":"ask"}}"""), + ("agent_thought_chunk", """{"sessionUpdate":"agent_thought_chunk","messageId":"m-3","content":{"type":"text","text":"think"}}"""), ("tool_call_update", """{"sessionUpdate":"tool_call_update","toolCallId":"tc-1","status":"completed"}"""), ("available_commands_update", """{"sessionUpdate":"available_commands_update","availableCommands":[]}"""), ("config_option_update", """{"sessionUpdate":"config_option_update","configOptions":[]}"""), diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireCommandInputTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireCommandInputTests.cs new file mode 100644 index 000000000..e7bb80036 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireCommandInputTests.cs @@ -0,0 +1,92 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class V2WireCommandInputTests +{ + [Theory] + [InlineData("{}")] + [InlineData("false")] + [InlineData("{\"hint\":\"value\"}")] + [InlineData("{\"type\":null,\"hint\":\"value\"}")] + [InlineData("{\"type\":42,\"hint\":\"value\"}")] + [InlineData("{\"type\":\"text\"}")] + [InlineData("{\"type\":\"text\",\"hint\":false}")] + public void AvailableCommandInputV2_InvalidInput_RejectsTheRootAndDefaultsTheOptionalParent(string inputJson) + { + // Arrange + var commandJson = $$"""{"name":"review","description":"Review","input":{{inputJson}}}"""; + + // Act + var command = Assert.IsType(JsonSerializer.Deserialize(commandJson, Wire.V2())); + var list = JsonSerializer.Deserialize($"[{commandJson}]", Wire.V2>()); + var update = Assert.IsType(JsonSerializer.Deserialize( + $$$"""{"sessionId":"session","update":{"sessionUpdate":"available_commands_update","availableCommands":[{{{commandJson}}}]}}""", + Wire.V2())); + + // Assert + Assert.Throws(() => JsonSerializer.Deserialize(inputJson, Wire.V2())); + Assert.Null(command.Input); + Assert.Null(Assert.Single(Assert.IsType>(list)).Input); + Assert.Null(Assert.Single(Assert.IsType(update.Update).AvailableCommands).Input); + } + + [Theory] + [InlineData("\"future\"")] + [InlineData("false")] + [InlineData("null")] + public void AvailableCommandInputV1_UnknownTypeProperty_RemainsMetadata(string typeJson) + { + // Arrange + var json = $$$"""{"type":{{{typeJson}}},"hint":"branch","vendor":{"format":1.20e+02}}"""; + + // Act + var input = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V1())); + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(input, Wire.V1())); + + // Assert + Assert.Equal("branch", input.Hint); + using var expected = JsonDocument.Parse(json); + Assert.True(JsonElement.DeepEquals(expected.RootElement, replay.RootElement)); + } + + [Fact] + public void AvailableCommandInputV2_UnknownDiscriminator_PreservesEveryParentBoundary() + { + // Arrange + const string inputJson = """{"type":"_future","schema":{"fields":[1.20e+02]},"_meta":{"vendor":false}}"""; + var commandJson = $$"""{"name":"review","description":"Review","input":{{inputJson}}}"""; + var updateJson = $$$"""{"sessionId":"session","update":{"sessionUpdate":"available_commands_update","availableCommands":[{{{commandJson}}}]}}"""; + + // Act + var direct = Assert.IsType(JsonSerializer.Deserialize(inputJson, Wire.V2())); + var parent = Assert.IsType(JsonSerializer.Deserialize(updateJson, Wire.V2())); + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(parent, Wire.V2())); + + // Assert + Assert.Equal(inputJson, JsonSerializer.Serialize(direct, Wire.V2())); + Assert.Equal(inputJson, replay.RootElement.GetProperty("update").GetProperty("availableCommands")[0].GetProperty("input").GetRawText()); + } + + [Fact] + public void AvailableCommandInput_ArbitraryHint_RoundTripsOnBothVersions() + => FsCheckPropertyRunner.Run(this, nameof(CommandHintRoundTripProperty)); + + private void CommandHintRoundTripProperty(string? hint) + { + // Arrange + var input = new AvailableCommandInput { Hint = hint ?? string.Empty }; + var command = new AvailableCommand { Name = "review", Description = "Review", Input = input }; + + foreach (var version in new[] { AcpProtocolVersion.V1, AcpProtocolVersion.V2 }) + { + // Act + var json = JsonSerializer.Serialize(command, Wire.Of(version)); + var restored = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + + // Assert + Assert.Equal(input.Hint, Assert.IsType(restored.Input).Hint); + } + } +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs new file mode 100644 index 000000000..c7b377e92 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs @@ -0,0 +1,327 @@ +using System.Text.Json; +using SalmonEgg.Acp.Content; +using SalmonEgg.Acp.Mcp; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class V2WireContractTests +{ + [Theory] + [InlineData(AcpProtocolVersion.V1, "group", "id")] + [InlineData(AcpProtocolVersion.V2, "groupId", "configId")] + public void ConfigOptionGroup_NegotiatedVersion_UsesItsOwnIdentifierAtEveryRoot(int version, string groupId, string configId) + { + // Arrange + var groupJson = $$"""{"{{groupId}}":"models","name":"Models","options":[{"value":"fast","name":"Fast"}]}"""; + var configJson = $$"""{"{{configId}}":"model","name":"Model","type":"select","currentValue":"fast","options":[{{groupJson}}]}"""; + + // Act + var group = Assert.IsType(JsonSerializer.Deserialize(groupJson, Wire.Of(version))); + var config = Assert.IsType(JsonSerializer.Deserialize(configJson, Wire.Of(version))); + using var direct = JsonDocument.Parse(JsonSerializer.Serialize(group, Wire.Of(version))); + using var list = JsonDocument.Parse(JsonSerializer.Serialize(new List { group }, Wire.Of>(version))); + using var parent = JsonDocument.Parse(JsonSerializer.Serialize(config, Wire.Of(version))); + + // Assert + Assert.Equal("models", group.Group); + Assert.Equal("models", Assert.Single(config.OptionGroups).Group); + using var expected = JsonDocument.Parse(groupJson); + Assert.True(JsonElement.DeepEquals(expected.RootElement, direct.RootElement)); + Assert.True(JsonElement.DeepEquals(expected.RootElement, list.RootElement[0])); + Assert.True(JsonElement.DeepEquals(expected.RootElement, parent.RootElement.GetProperty("options")[0])); + } + + [Theory] + [InlineData("agent_message_chunk")] + [InlineData("user_message_chunk")] + [InlineData("agent_thought_chunk")] + public void ContentChunkV2_MissingMessageId_RejectsEveryEnvelope(string discriminator) + { + // Arrange + var json = $$$"""{"sessionUpdate":"{{{discriminator}}}","content":{"type":"text","text":"hello"}}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize($"[{json}]", Wire.V2>())); + Assert.Throws(() => JsonSerializer.Deserialize( + $$"""{"sessionId":"s","update":{{json}}}""", Wire.V2())); + } + + [Theory] + [InlineData("agent_message")] + [InlineData("user_message")] + [InlineData("agent_thought")] + public void WholeMessageV2_MissingMessageId_RejectsEveryEnvelope(string discriminator) + { + // Arrange + var json = $$"""{"sessionUpdate":"{{discriminator}}","content":[]}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize($"[{json}]", Wire.V2>())); + Assert.Throws(() => JsonSerializer.Deserialize( + $$"""{"sessionId":"s","update":{{json}}}""", Wire.V2())); + } + + [Fact] + public void WholeMessageV2_NullMessageId_RejectsReadAndWrite() + { + // Arrange + var update = new AgentWholeMessageUpdate { MessageId = null! }; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize("""{"messageId":null}""", Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(update, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(new SessionUpdateParams { SessionId = "s", Update = update }, Wire.V2())); + } + + [Theory] + [InlineData("null")] + [InlineData("42")] + public void ContentChunkV2_NonStringMessageId_RejectsDirectRoot(string rawValue) + { + // Arrange + var json = $$$"""{"messageId":{{{rawValue}}},"content":{"type":"text","text":"hello"}}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); + } + + [Fact] + public void ContentChunkV2_MissingMessageId_RejectsWritingEveryRoot() + { + // Arrange + var chunk = new AgentMessageUpdate(new TextContentBlock("hello")); + + // Act / Assert + Assert.Throws(() => JsonSerializer.Serialize(chunk, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(chunk, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(new List { chunk }, Wire.V2>())); + Assert.Throws(() => JsonSerializer.Serialize( + new SessionUpdateParams { SessionId = "s", Update = chunk }, Wire.V2())); + } + + [Theory] + [InlineData("")] + [InlineData(",\"messageId\":null")] + [InlineData(",\"messageId\":42")] + public void ContentChunkV1_OptionalOrInvalidMessageId_UsesSchemaDefault(string messageProperty) + { + // Arrange + var json = $$$"""{"content":{"type":"text","text":"hello"}{{{messageProperty}}}}"""; + + // Act + var chunk = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V1())); + + // Assert + Assert.Null(chunk.MessageId); + Assert.NotNull(chunk.Content); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1)] + [InlineData(AcpProtocolVersion.V2)] + public void ResourceLink_Icons_PreservesRawMetadataAtEveryRoot(int version) + { + // Arrange + const string json = """{"type":"resource_link","uri":"https://example.test/doc","name":"Doc","icons":[{"src":"https://example.test/icon.svg","theme":"future","vendor":{"size":1.20e+02}}]}"""; + + // Act + var direct = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + var parent = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + var list = JsonSerializer.Deserialize($"[{json}]", Wire.Of>(version)); + var replay = new[] + { + JsonSerializer.Serialize(direct, Wire.Of(version)), + JsonSerializer.Serialize(parent, Wire.Of(version)), + JsonSerializer.Serialize(Assert.Single(list!), Wire.Of(version)) + }; + + // Assert + using var expected = JsonDocument.Parse(json); + foreach (var value in replay) + { + using var actual = JsonDocument.Parse(value); + Assert.Equal(expected.RootElement.GetProperty("icons").GetRawText(), actual.RootElement.GetProperty("icons").GetRawText()); + } + } + + [Theory] + [InlineData(AcpProtocolVersion.V1)] + [InlineData(AcpProtocolVersion.V2)] + public void AvailableCommandInput_Text_UsesVersionedDiscriminatorAtEveryRoot(int version) + { + // Arrange + var input = new AvailableCommandInput { Hint = "branch name" }; + var command = new AvailableCommand { Name = "review", Description = "Review", Input = input }; + + // Act + using var direct = JsonDocument.Parse(JsonSerializer.Serialize(input, Wire.Of(version))); + using var parent = JsonDocument.Parse(JsonSerializer.Serialize(command, Wire.Of(version))); + using var list = JsonDocument.Parse(JsonSerializer.Serialize(new List { command }, Wire.Of>(version))); + + // Assert + Assert.Equal("branch name", direct.RootElement.GetProperty("hint").GetString()); + Assert.True(JsonElement.DeepEquals(direct.RootElement, parent.RootElement.GetProperty("input"))); + Assert.True(JsonElement.DeepEquals(direct.RootElement, list.RootElement[0].GetProperty("input"))); + Assert.Equal(version == AcpProtocolVersion.V2, direct.RootElement.TryGetProperty("type", out var type)); + if (version == AcpProtocolVersion.V2) Assert.Equal("text", type.GetString()); + } + + [Theory] + [InlineData("_vendor_form")] + [InlineData("future_form")] + public void AvailableCommandInputV2_UnknownType_RoundTripsWithoutInventingText(string type) + { + // Arrange + var json = $$$$"""{"name":"review","description":"Review","input":{"type":"{{{{type}}}}","schema":{"fields":[1.20e+02]}}}"""; + + // Act + var command = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var replay = JsonSerializer.Serialize(command, Wire.V2()); + + // Assert + using var expected = JsonDocument.Parse(json); + using var actual = JsonDocument.Parse(replay); + Assert.Equal(expected.RootElement.GetProperty("input").GetRawText(), actual.RootElement.GetProperty("input").GetRawText()); + } + + [Fact] + public void TextCommandInput_DraftWire_UsesTextDiscriminator() + { + // Arrange + var input = new TextCommandInput { Hint = "branch name" }; + + // Act + using var json = JsonDocument.Parse(JsonSerializer.Serialize(input, Wire.V2())); + + // Assert + Assert.Equal("text", json.RootElement.GetProperty("type").GetString()); + } + + [Fact] + public void McpServerV2_Sse_RemainsUnknownWithRawPayload() + { + // Arrange + const string json = """{"type":"sse","name":"events","url":"https://example.test/events","future":{"encoding":1.20e+02}}"""; + + // Act + var server = JsonSerializer.Deserialize(json, Wire.V2()); + var setup = JsonSerializer.Deserialize($$"""{"cwd":"/tmp","mcpServers":[{{json}}]}""", Wire.V2()); + + // Assert + Assert.IsType(server); + Assert.IsType(Assert.Single(setup!.McpServers)); + Assert.Equal(json, JsonSerializer.Serialize(server, Wire.V2())); + } + + [Theory] + [InlineData("")] + [InlineData(",\"type\":null")] + [InlineData(",\"type\":42")] + public void McpServerV2_MissingOrWrongDiscriminator_RejectsInsteadOfDefaultingToStdio(string typeProperty) + { + // Arrange + var json = $$"""{"name":"mcp","command":"mcp"{{typeProperty}}}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize( + $$"""{"cwd":"/tmp","mcpServers":[{{json}}]}""", Wire.V2())); + } + + [Fact] + public void SessionResponsesV2_Modes_DoNotReadOrWriteV1State() + { + // Arrange + const string json = """{"sessionId":"s","modes":{"currentModeId":"plan","availableModes":[]}}"""; + var modes = new SessionModesState { CurrentModeId = "plan" }; + + // Act + var created = JsonSerializer.Deserialize(json, Wire.V2()); + var resumed = JsonSerializer.Deserialize(json, Wire.V2()); + using var createdJson = JsonDocument.Parse(JsonSerializer.Serialize(new SessionNewResponse("s", modes), Wire.V2())); + using var resumedJson = JsonDocument.Parse(JsonSerializer.Serialize(new SessionResumeResponse(modes), Wire.V2())); + + // Assert + Assert.Null(created!.Modes); + Assert.Null(resumed!.Modes); + Assert.False(createdJson.RootElement.TryGetProperty("modes", out _)); + Assert.False(resumedJson.RootElement.TryGetProperty("modes", out _)); + } + + [Fact] + public void AgentAuthCapabilitiesV2_LogoutMarker_DoesNotAdvertiseV1Capability() + { + // Arrange + const string json = """{"logout":{}}"""; + var capabilities = new AgentAuthCapabilities { Logout = new LogoutCapabilities() }; + + // Act + var auth = JsonSerializer.Deserialize(json, Wire.V2()); + var response = JsonSerializer.Deserialize( + """{"protocolVersion":2,"info":{"name":"agent","version":"1"},"capabilities":{"auth":{"logout":{}}}}""", + AcpJsonContext.Default.InitializeResponse); + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(capabilities, Wire.V2())); + + // Assert + Assert.Null(auth!.Logout); + Assert.False(response!.AgentCapabilities.SupportsLogout); + Assert.False(replay.RootElement.TryGetProperty("logout", out _)); + } + + [Theory] + [InlineData("")] + [InlineData(",\"info\":null")] + [InlineData(",\"info\":{}")] + [InlineData(",\"info\":{\"name\":\"agent\"}")] + [InlineData(",\"info\":{\"name\":false,\"version\":\"1\"}")] + public void InitializeV2_MissingOrInvalidRequiredInfo_RejectsBothDirections(string infoProperty) + { + // Arrange + var json = $$"""{"protocolVersion":2{{infoProperty}}}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeParams)); + Assert.Throws(() => JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeResponse)); + } + + [Theory] + [InlineData("")] + [InlineData(",\"capabilities\":null")] + [InlineData(",\"capabilities\":false")] + public void InitializeV2_DefaultableCapabilities_UsesEmptyCapabilities(string capabilitiesProperty) + { + // Arrange + var json = $$$"""{"protocolVersion":2,"info":{"name":"peer","version":"1"}{{{capabilitiesProperty}}}}"""; + + // Act + var request = JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeParams); + var response = JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeResponse); + + // Assert + Assert.NotNull(request!.ClientCapabilities); + Assert.False(response!.AgentCapabilities.SupportsLogout); + Assert.False(response.AgentCapabilities.SupportsSessionList); + } + + [Theory] + [InlineData("")] + [InlineData(",\"configOptions\":null")] + [InlineData(",\"configOptions\":false")] + public void SessionResponseV2_DefaultableConfigOptions_UsesAnEmptyList(string configProperty) + { + // Arrange + var json = $$"""{"sessionId":"s"{{configProperty}}}"""; + + // Act + var created = JsonSerializer.Deserialize(json, Wire.V2()); + var resumed = JsonSerializer.Deserialize(json, Wire.V2()); + + // Assert + Assert.Empty(Assert.IsType>(created!.ConfigOptions)); + Assert.Empty(Assert.IsType>(resumed!.ConfigOptions)); + } +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireDefaultValueTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireDefaultValueTests.cs new file mode 100644 index 000000000..80a1ea1f0 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireDefaultValueTests.cs @@ -0,0 +1,223 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class V2WireDefaultValueTests +{ + [Theory] + [InlineData("null")] + [InlineData("false")] + [InlineData("42")] + [InlineData("{}")] + public void ConfigOptionsV2_InvalidArrayValue_DefaultsAtEveryParent(string value) + { + // Arrange + var json = $$"""{"sessionId":"session","configOptions":{{value}}}"""; + + // Act + var created = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var resumed = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var configured = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var update = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + + // Assert + Assert.Empty(Assert.IsType>(created.ConfigOptions)); + Assert.Empty(Assert.IsType>(resumed.ConfigOptions)); + Assert.Empty(Assert.IsType>(configured.ConfigOptions)); + Assert.Empty(Assert.IsType>(update.ConfigOptions)); + } + + [Fact] + public void ConfigOptionsV2_MissingRequiredArray_RejectsUpdateAndSetResponse() + { + // Arrange / Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize("{}", Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize("{}", Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize( + """{"sessionId":"session","update":{"sessionUpdate":"config_option_update"}}""", Wire.V2())); + } + + [Fact] + public void ConfigOptionsV2_MissingAuthoredState_RejectsWritingARequiredArray() + { + // Arrange + var update = new ConfigOptionUpdate(); + + // Act / Assert + Assert.Throws(() => JsonSerializer.Serialize(new SessionSetConfigOptionResponse(), Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(update, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(new SessionUpdateParams { SessionId = "session", Update = update }, Wire.V2())); + } + + [Fact] + public void InitializeV2_MissingAuthoredInfo_RejectsBothDirections() + { + // Arrange + var request = new InitializeParams { ProtocolVersion = AcpProtocolVersion.V2, ClientInfo = null! }; + var response = new InitializeResponse { ProtocolVersion = AcpProtocolVersion.V2, AgentInfo = null! }; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Serialize(request, AcpJsonContext.Default.InitializeParams)); + Assert.Throws(() => JsonSerializer.Serialize(response, AcpJsonContext.Default.InitializeResponse)); + Assert.Throws(() => JsonSerializer.Serialize(request with { ClientInfo = new ClientInfo { Name = null! } }, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(response with { AgentInfo = new AgentInfo { Version = null! } }, Wire.V2())); + } + + [Fact] + public void ContentChunkV1_DefaultSourceGeneratedContract_UsesTheSameSchemaDefault() + { + // Arrange + const string json = """{"content":{"type":"text","text":"hello"},"messageId":false}"""; + + // Act + var chunk = Assert.IsType(JsonSerializer.Deserialize(json, AcpJsonContext.Default.AgentMessageUpdate)); + + // Assert + Assert.Null(chunk.MessageId); + } + + [Fact] + public void ConfigOptionsV2_InvalidItems_KeepValidAndUnknownSuccessors() + { + // Arrange + const string unknown = """{"configId":"future","name":"Future","type":"_future","payload":{"size":1.20e+02}}"""; + var json = $$$""" + {"sessionId":"session","configOptions":[ + {},null,false,{"configId":"broken","name":"Broken","type":"boolean","currentValue":"wrong"}, + {"configId":"enabled","name":"Enabled","type":"boolean","currentValue":true},{{{unknown}}} + ]} + """; + + // Act + var created = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var resumed = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var configured = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + var update = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + + // Assert + foreach (var options in new[] { created.ConfigOptions, resumed.ConfigOptions, configured.ConfigOptions, update.ConfigOptions }) + { + var items = Assert.IsType>(options); + Assert.Equal(new[] { "enabled", "future" }, items.Select(option => option.Id)); + Assert.Equal(unknown, JsonSerializer.Serialize(items[1], Wire.V2())); + } + + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(created, Wire.V2())); + Assert.Equal(unknown, replay.RootElement.GetProperty("configOptions")[1].GetRawText()); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "group")] + [InlineData(AcpProtocolVersion.V2, "groupId")] + public void ConfigGroup_DefaultableOptions_KeepTheValidSuccessor(int version, string groupId) + { + // Arrange + var json = $$"""{"{{groupId}}":"models","name":"Models","options":[false,{},{"value":"fast","name":"Fast","description":42}]}"""; + + // Act + var group = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + + // Assert + var option = Assert.Single(group.Options); + Assert.Equal("fast", option.Value); + Assert.Null(option.Description); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "group")] + [InlineData(AcpProtocolVersion.V2, "groupId")] + public void ConfigGroup_MissingAndInvalidOptions_RespectRequiredAndDefaultableContracts(int version, string groupId) + { + // Arrange + var missing = $$"""{"{{groupId}}":"models","name":"Models"}"""; + var invalid = $$"""{"{{groupId}}":"models","name":"Models","options":false}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(missing, Wire.Of(version))); + var group = Assert.IsType(JsonSerializer.Deserialize(invalid, Wire.Of(version))); + Assert.Empty(group.Options); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "id", "configId")] + [InlineData(AcpProtocolVersion.V2, "configId", "id")] + public void ConfigOption_ConflictingVersionIdentifiers_PreservesUnknownFieldWithoutChangingIdentity(int version, string id, string unknownId) + { + // Arrange + var json = $$"""{"{{id}}":"current","{{unknownId}}":"other","name":"Current","type":"boolean","currentValue":true}"""; + + // Act + var option = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(option, Wire.Of(version))); + + // Assert + Assert.Equal("current", option.Id); + Assert.Equal("other", replay.RootElement.GetProperty(unknownId).GetString()); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "configId")] + [InlineData(AcpProtocolVersion.V2, "id")] + public void ConfigOption_MissingVersionIdentifier_RejectsTheOtherVersionAlias(int version, string otherId) + { + // Arrange + var json = $$"""{"{{otherId}}":"current","name":"Current","type":"boolean","currentValue":true}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.Of(version))); + } + + [Theory] + [InlineData("")] + [InlineData(",\"configOptions\":null")] + public void SessionResponsesV1_OptionalConfigOptions_RemainsUnprovided(string property) + { + // Arrange + var json = $$"""{"sessionId":"session"{{property}}}"""; + + // Act + var created = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V1())); + var resumed = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V1())); + + // Assert + Assert.Null(created.ConfigOptions); + Assert.Null(resumed.ConfigOptions); + } + + [Fact] + public void ClientCapabilitiesV2_LegacyFields_AreIgnoredOnReadAndRejectedOnWrite() + { + // Arrange + const string capabilities = """{"fs":{"readTextFile":true},"terminal":true,"session":{"configOptions":{}}}"""; + var json = $$"""{"protocolVersion":2,"info":{"name":"client","version":"1"},"capabilities":{{capabilities}}}"""; + var legacy = new ClientCapabilities(fs: new FsCapability(), terminal: true); + + // Act + var direct = Assert.IsType(JsonSerializer.Deserialize(capabilities, Wire.V2())); + var parent = Assert.IsType(JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeParams)); + + // Assert + Assert.Null(direct.Fs); + Assert.Null(direct.Terminal); + Assert.Null(direct.Session); + Assert.Equal(direct, parent.ClientCapabilities); + Assert.Throws(() => JsonSerializer.Serialize(legacy, Wire.V2())); + Assert.Throws(() => JsonSerializer.Serialize(new InitializeParams { ProtocolVersion = 2, ClientCapabilities = legacy }, AcpJsonContext.Default.InitializeParams)); + } + + [Theory] + [InlineData("{\"name\":\"peer\",\"version\":null}")] + [InlineData("[]")] + [InlineData("false")] + public void InitializeV2_InvalidInfo_RejectsBothRoots(string info) + { + // Arrange + var json = $$"""{"protocolVersion":2,"info":{{info}}}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeParams)); + Assert.Throws(() => JsonSerializer.Deserialize(json, AcpJsonContext.Default.InitializeResponse)); + } +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireResourceLinkTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireResourceLinkTests.cs new file mode 100644 index 000000000..10bd8fae9 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireResourceLinkTests.cs @@ -0,0 +1,116 @@ +using System.Text.Json; +using SalmonEgg.Acp.Content; +using SalmonEgg.Acp.Protocol; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class V2WireResourceLinkTests +{ + [Theory] + [InlineData("")] + [InlineData(",\"icons\":null")] + [InlineData(",\"icons\":false")] + [InlineData(",\"icons\":{}")] + public void GetIcons_DefaultableMetadata_ReturnsNoIcons(string property) + { + // Arrange + var json = $$"""{"type":"resource_link","uri":"https://example.test/doc","name":"Doc"{{property}}}"""; + var resource = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + + // Act / Assert + Assert.Empty(resource.GetIcons()); + } + + [Fact] + public void GetIcons_InvalidEntries_KeepTheValidSuccessorAndUnknownFields() + { + // Arrange + const string json = """ + {"type":"resource_link","uri":"https://example.test/doc","name":"Doc","icons":[ + {},null,42,{"src":null},{"src":false}, + {"src":"https://example.test/icon.svg","sizes":[false,"any",null,"48x48"],"mimeType":42,"theme":"future","vendor":{"size":1.20e+02}} + ]} + """; + var resource = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + + // Act + var icon = Assert.Single(resource.GetIcons()); + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(icon, Wire.V2())); + + // Assert + Assert.Equal("https://example.test/icon.svg", icon.Src); + Assert.Equal(new[] { "any", "48x48" }, icon.Sizes); + Assert.Null(icon.MimeType); + Assert.Equal("future", icon.Theme); + Assert.Equal("{\"size\":1.20e+02}", replay.RootElement.GetProperty("vendor").GetRawText()); + } + + [Fact] + public void WithIcons_DraftMetadata_UsesTheSameWireAtEveryRoot() + { + // Arrange + var resource = new ResourceLinkContentBlock("https://example.test/doc", "Doc"); + var updated = resource.WithIcons([new Icon { Src = "https://example.test/icon.svg", Theme = "future" }]); + var prompt = new SessionPromptParams("session", [updated]); + + // Act + using var direct = JsonDocument.Parse(JsonSerializer.Serialize(updated, Wire.V2())); + using var content = JsonDocument.Parse(JsonSerializer.Serialize(updated, Wire.V2())); + using var list = JsonDocument.Parse(JsonSerializer.Serialize(new List { updated }, Wire.V2>())); + using var parent = JsonDocument.Parse(JsonSerializer.Serialize(prompt, Wire.V2())); + var restored = Assert.IsType(JsonSerializer.Deserialize(parent.RootElement, Wire.V2())); + + // Assert + Assert.Empty(resource.GetIcons()); + Assert.Equal("future", Assert.Single(updated.GetIcons()).Theme); + Assert.True(JsonElement.DeepEquals(direct.RootElement, content.RootElement)); + Assert.True(JsonElement.DeepEquals(direct.RootElement, list.RootElement[0])); + Assert.True(JsonElement.DeepEquals(direct.RootElement, parent.RootElement.GetProperty("prompt")[0])); + Assert.Equal("future", Assert.Single(Assert.IsType(Assert.Single(restored.Prompt)).GetIcons()).Theme); + } + + [Fact] + public void WithIcons_StableWire_RejectsDraftMetadataAtEveryRoot() + { + // Arrange + var resource = new ResourceLinkContentBlock("https://example.test/doc", "Doc") + .WithIcons([new Icon { Src = "https://example.test/icon.svg" }]); + + // Act / Assert + Assert.Throws(() => JsonSerializer.Serialize(resource, Wire.V1())); + Assert.Throws(() => JsonSerializer.Serialize(resource, Wire.V1())); + Assert.Throws(() => JsonSerializer.Serialize(new List { resource }, Wire.V1>())); + Assert.Throws(() => JsonSerializer.Serialize(new SessionPromptParams("session", [resource]), Wire.V1())); + } + + [Theory] + [InlineData("{}")] + [InlineData("[]")] + [InlineData("false")] + [InlineData("{\"src\":null}")] + [InlineData("{\"src\":42}")] + public void Icon_InvalidRequiredSource_RejectsTheDirectContract(string json) + { + // Arrange / Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); + } + + [Fact] + public void WithIcons_ArbitraryTheme_PreservesTheValueInAParentPrompt() + => FsCheckPropertyRunner.Run(this, nameof(IconThemeRoundTripProperty)); + + private void IconThemeRoundTripProperty(string? theme) + { + // Arrange + var resource = new ResourceLinkContentBlock("https://example.test/doc", "Doc") + .WithIcons([new Icon { Src = "https://example.test/icon.svg", Theme = theme }]); + + // Act + var json = JsonSerializer.Serialize(new SessionPromptParams("session", [resource]), Wire.V2()); + var restored = Assert.IsType(JsonSerializer.Deserialize(json, Wire.V2())); + + // Assert + var restoredResource = Assert.IsType(Assert.Single(restored.Prompt)); + Assert.Equal(theme, Assert.Single(restoredResource.GetIcons()).Theme); + } +} From 97ffb187a37039bb408bb391b0681bcd20a56222 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Tue, 8 Sep 2026 19:48:51 +0000 Subject: [PATCH 2/6] fix(acp): preserve nested fields and default capability errors --- .../Protocol/AuthCapabilityTypes.cs | 84 +++++++ .../Protocol/ConfigOptionTypes.cs | 55 +++-- src/SalmonEgg.Acp/Protocol/InitializeTypes.cs | 25 +-- src/SalmonEgg.Acp/PublicSurface.Types.txt | 1 + .../Serialization/AcpJsonContext.cs | 1 + .../Serialization/AcpWireFormat.cs | 10 + .../DefaultableProtocolJsonConverters.cs | 26 +++ .../Protocol/AuthCapabilityTypesTests.cs | 103 +++++++++ .../Protocol/V2WireReviewBoundaryTests.cs | 211 ++++++++++++++++++ 9 files changed, 486 insertions(+), 30 deletions(-) create mode 100644 src/SalmonEgg.Acp/Protocol/AuthCapabilityTypes.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/AuthCapabilityTypesTests.cs create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs diff --git a/src/SalmonEgg.Acp/Protocol/AuthCapabilityTypes.cs b/src/SalmonEgg.Acp/Protocol/AuthCapabilityTypes.cs new file mode 100644 index 000000000..680254e64 --- /dev/null +++ b/src/SalmonEgg.Acp/Protocol/AuthCapabilityTypes.cs @@ -0,0 +1,84 @@ +using System; +using System.Text.Json; +using System.Text.Json.Serialization; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Protocol; + +/// Opt-in authentication method types the client can execute. +[JsonConverter(typeof(AuthCapabilitiesJsonConverter))] +public sealed record AuthCapabilities : AcpProtocolObject +{ + /// + /// Whether the client can reproduce the configured agent invocation in an interactive terminal. + /// This is a boolean in v1 and a presence marker in v2. An empty auth object advertises no support. + /// + [JsonPropertyName("terminal")] + public bool Terminal { get; init; } + + internal JsonElement? RawPayload { get; init; } +} + +internal sealed class AuthCapabilitiesJsonConverter : JsonConverter +{ + public override AuthCapabilities? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + var root = document.RootElement; + if (root.ValueKind != JsonValueKind.Object) + { + throw new JsonException("ACP authentication capabilities must be an object."); + } + + var terminal = root.TryGetProperty("terminal", out var value) + && (AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2 + ? value.ValueKind == JsonValueKind.Object + : value.ValueKind == JsonValueKind.True); + + return new AuthCapabilities + { + Terminal = terminal, + Meta = root.TryGetProperty("_meta", out var meta) && meta.ValueKind == JsonValueKind.Object + ? AcpMetaJson.Read(root) : null, + RawPayload = root.Clone() + }; + } + + public override void Write(Utf8JsonWriter writer, AuthCapabilities value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + if (AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V1) + { + writer.WriteBoolean("terminal", value.Terminal); + } + else if (value.Terminal) + { + writer.WritePropertyName("terminal"); + if (value.RawPayload is { } raw && raw.TryGetProperty("terminal", out var terminal) + && terminal.ValueKind == JsonValueKind.Object) + { + writer.WriteRawValue(terminal.GetRawText()); + } + else + { + writer.WriteStartObject(); + writer.WriteEndObject(); + } + } + + AcpMetaJson.Write(writer, value.Meta); + if (value.RawPayload is { } payload) + { + foreach (var property in payload.EnumerateObject()) + { + if (property.Name is not "terminal" and not "_meta") + { + writer.WritePropertyName(property.Name); + writer.WriteRawValue(property.Value.GetRawText()); + } + } + } + + writer.WriteEndObject(); + } +} diff --git a/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs b/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs index 4e9fbdadd..1577ad7db 100644 --- a/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/ConfigOptionTypes.cs @@ -42,6 +42,7 @@ public sealed record ConfigOption : AcpProtocolObject internal JsonElement? RawPayload { get; init; } } +[JsonConverter(typeof(ConfigOptionValueJsonConverter))] public sealed record ConfigOptionValue : AcpProtocolObject { [JsonPropertyName("value")] @@ -52,6 +53,8 @@ public sealed record ConfigOptionValue : AcpProtocolObject [JsonPropertyName("description")] public string? Description { get; init; } + + internal JsonElement? RawPayload { get; init; } } [JsonConverter(typeof(ConfigOptionGroupJsonConverter))] @@ -65,6 +68,8 @@ public sealed record ConfigOptionGroup : AcpProtocolObject [JsonPropertyName("options")] public List Options { get; init; } = new(); + + internal JsonElement? RawPayload { get; init; } } internal sealed class ConfigOptionJsonConverter : JsonConverter @@ -169,7 +174,8 @@ public override void Write(Utf8JsonWriter writer, ConfigOption value, JsonSerial } AcpMetaJson.Write(writer, value.Meta); - WriteUnknownFields(writer, value, options); + WriteUnknownFields(writer, value.RawPayload, + IdPropertyName(options), "name", "description", "category", "type", "currentValue", "options", "_meta"); writer.WriteEndObject(); } @@ -204,7 +210,8 @@ internal static ConfigOptionGroup ReadGroup(JsonElement element, JsonSerializerO Group = ReadRequiredString(element, GroupPropertyName(options)), Name = ReadRequiredString(element, "name"), Options = groupOptions, - Meta = AcpMetaJson.Read(element) + Meta = AcpMetaJson.Read(element), + RawPayload = element.Clone() }; } @@ -216,6 +223,7 @@ internal static void WriteGroup(Utf8JsonWriter writer, ConfigOptionGroup group, writer.WritePropertyName("options"); WriteOptions(writer, group.Options, options); AcpMetaJson.Write(writer, group.Meta); + WriteUnknownFields(writer, group.RawPayload, GroupPropertyName(options), "name", "options", "_meta"); writer.WriteEndObject(); } @@ -257,15 +265,27 @@ private static void ReadSelectOptions( } } - private static ConfigOptionValue ReadOption(JsonElement element) + internal static ConfigOptionValue ReadOption(JsonElement element) => new() { Value = ReadRequiredString(element, "value"), Name = ReadRequiredString(element, "name"), Description = ReadOptionalString(element, "description"), - Meta = AcpMetaJson.Read(element) + Meta = AcpMetaJson.Read(element), + RawPayload = element.Clone() }; + internal static void WriteOption(Utf8JsonWriter writer, ConfigOptionValue value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString("value", value.Value); + writer.WriteString("name", value.Name); + WriteOptionalString(writer, "description", value.Description, options); + AcpMetaJson.Write(writer, value.Meta); + WriteUnknownFields(writer, value.RawPayload, "value", "name", "description", "_meta"); + writer.WriteEndObject(); + } + private static string IdPropertyName(JsonSerializerOptions options) => AcpWireFormat.NegotiatedVersion(options) == AcpProtocolVersion.V2 ? "configId" : "id"; @@ -292,18 +312,16 @@ private static string ReadRequiredString(JsonElement root, string propertyName) : null; } - private static void WriteUnknownFields(Utf8JsonWriter writer, ConfigOption value, JsonSerializerOptions options) + private static void WriteUnknownFields(Utf8JsonWriter writer, JsonElement? rawPayload, params string[] knownPropertyNames) { - if (value.RawPayload is not { } payload) + if (rawPayload is not { } payload) { return; } - var idProperty = IdPropertyName(options); foreach (var property in payload.EnumerateObject()) { - if (property.Name != idProperty && property.Name is not "name" and not "description" and not "category" - and not "type" and not "currentValue" and not "options" and not "_meta") + if (System.Array.IndexOf(knownPropertyNames, property.Name) < 0) { writer.WritePropertyName(property.Name); writer.WriteRawValue(property.Value.GetRawText()); @@ -333,12 +351,7 @@ private static void WriteOptions( writer.WriteStartArray(); foreach (var option in configOptions) { - writer.WriteStartObject(); - writer.WriteString("value", option.Value); - writer.WriteString("name", option.Name); - WriteOptionalString(writer, "description", option.Description, serializerOptions); - AcpMetaJson.Write(writer, option.Meta); - writer.WriteEndObject(); + WriteOption(writer, option, serializerOptions); } writer.WriteEndArray(); @@ -362,6 +375,18 @@ private static void WriteOptionalString( } } +internal sealed class ConfigOptionValueJsonConverter : JsonConverter +{ + public override ConfigOptionValue? Read(ref Utf8JsonReader reader, System.Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + return ConfigOptionJsonConverter.ReadOption(document.RootElement); + } + + public override void Write(Utf8JsonWriter writer, ConfigOptionValue value, JsonSerializerOptions options) + => ConfigOptionJsonConverter.WriteOption(writer, value, options); +} + internal sealed class ConfigOptionGroupJsonConverter : JsonConverter { public override ConfigOptionGroup? Read(ref Utf8JsonReader reader, System.Type typeToConvert, JsonSerializerOptions options) diff --git a/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs b/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs index bcc411bc7..450c17307 100644 --- a/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/InitializeTypes.cs @@ -125,6 +125,13 @@ public sealed record ClientCapabilities : AcpProtocolObject [JsonPropertyName("session")] public ClientSessionCapabilities? Session { get; init; } + /// + /// Authentication-method extensions supported by the client. Omitted means no opt-in support. + /// + [JsonPropertyName("auth")] + [JsonConverter(typeof(DefaultableObjectJsonConverter))] + public AuthCapabilities? Auth { get; init; } + /// /// Elicitation capabilities, declaring which elicitation/create modes the agent may use. /// @@ -856,19 +863,7 @@ private static ClientCapabilities ReadClientCapabilities(JsonElement root, int p private static void WriteClientCapabilitiesV2(Utf8JsonWriter writer, ClientCapabilities value, JsonSerializerOptions options) { writer.WritePropertyName("capabilities"); - writer.WriteStartObject(); - - // elicitation is a root capability in the v2 schema too (unlike fs/terminal/session, which - // v2 dropped), so dropping it here would silently un-advertise a mode the client supports and - // make every standards-compliant agent fall back. - if (value.Elicitation is not null) - { - writer.WritePropertyName("elicitation"); - JsonSerializer.Serialize(writer, value.Elicitation, (JsonTypeInfo)options.GetTypeInfo(typeof(ElicitationCapabilities))); - } - - AcpMetaJson.Write(writer, value.Meta); - writer.WriteEndObject(); + JsonSerializer.Serialize(writer, value, AcpWireFormat.For(AcpProtocolVersion.V2).TypeInfo()); } } @@ -1017,9 +1012,9 @@ private static AgentCapabilities ReadAgentCapabilitiesV2(JsonElement root, JsonS sessionCapabilities = ReadSessionCapabilitiesV2(session); } - if (root.TryGetProperty("auth", out var authElement) && authElement.ValueKind == JsonValueKind.Object) + if (root.TryGetProperty("auth", out var authElement)) { - auth = JsonSerializer.Deserialize(authElement.GetRawText(), (JsonTypeInfo)options.GetTypeInfo(typeof(AgentAuthCapabilities))); + auth = DefaultableObjectJsonConverter.ReadValue(authElement, options); } return new AgentCapabilities diff --git a/src/SalmonEgg.Acp/PublicSurface.Types.txt b/src/SalmonEgg.Acp/PublicSurface.Types.txt index 6727373ca..c8e1039c8 100644 --- a/src/SalmonEgg.Acp/PublicSurface.Types.txt +++ b/src/SalmonEgg.Acp/PublicSurface.Types.txt @@ -79,6 +79,7 @@ SalmonEgg.Acp.Protocol.AskUserQuestion stable SalmonEgg.Acp.Protocol.AskUserRequest stable SalmonEgg.Acp.Protocol.AskUserRequestEventArgs stable SalmonEgg.Acp.Protocol.AskUserResponse stable +SalmonEgg.Acp.Protocol.AuthCapabilities stable SalmonEgg.Acp.Protocol.AuthMethod stable SalmonEgg.Acp.Protocol.AuthMethodDefinition stable SalmonEgg.Acp.Protocol.AuthenticateParams stable diff --git a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs index c4ecef0b7..b4b657126 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpJsonContext.cs @@ -35,6 +35,7 @@ namespace SalmonEgg.Acp.Serialization; [JsonSerializable(typeof(InitializeResponse))] [JsonSerializable(typeof(ClientInfo))] [JsonSerializable(typeof(ClientCapabilities))] +[JsonSerializable(typeof(AuthCapabilities))] [JsonSerializable(typeof(ClientSessionCapabilities))] [JsonSerializable(typeof(SessionConfigOptionsCapabilities))] [JsonSerializable(typeof(BooleanConfigOptionCapabilities))] diff --git a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs index faaa7793e..03f48d65b 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs @@ -145,8 +145,18 @@ private void ApplyObjectContract(JsonTypeInfo info) { IgnoreProperty(info, "logout", new IgnoredProtocolPropertyJsonConverter()); } + else if (info.Type == typeof(AgentCapabilities)) + { + FindProperty(info, "auth").CustomConverter = new DefaultableObjectJsonConverter(); + } + else if (info.Type == typeof(ElicitationCapabilities)) + { + FindProperty(info, "form").CustomConverter = new DefaultableObjectJsonConverter(); + FindProperty(info, "url").CustomConverter = new DefaultableObjectJsonConverter(); + } else if (info.Type == typeof(ClientCapabilities)) { + FindProperty(info, "elicitation").CustomConverter = new DefaultableObjectJsonConverter(); info.OnSerializing = static value => InitializeClientProtocolPolicy.Validate(AcpProtocolVersion.V2, (ClientCapabilities)value); IgnoreProperty(info, "fs", new IgnoredProtocolPropertyJsonConverter()); IgnoreProperty(info, "terminal", new IgnoredProtocolPropertyJsonConverter()); diff --git a/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs index a43af8316..f158c1e34 100644 --- a/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs +++ b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs @@ -36,6 +36,32 @@ public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOp => writer.WriteStringValue(value); } +internal sealed class DefaultableObjectJsonConverter : JsonConverter where T : class +{ + public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var document = JsonDocument.ParseValue(ref reader); + return ReadValue(document.RootElement, options); + } + + internal static T? ReadValue(JsonElement value, JsonSerializerOptions options) + { + try + { + return value.Deserialize((JsonTypeInfo)options.GetTypeInfo(typeof(T))); + } + catch (JsonException) + { + // Attach only to properties whose schema explicitly permits default-on-error. + // Typed root contracts remain strict, and valid sibling properties are retained. + return null; + } + } + + public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options) + => JsonSerializer.Serialize(writer, value, (JsonTypeInfo)options.GetTypeInfo(typeof(T))); +} + internal sealed class DefaultableConfigOptionsJsonConverter : JsonConverter> { public override bool HandleNull => true; diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/AuthCapabilityTypesTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/AuthCapabilityTypesTests.cs new file mode 100644 index 000000000..3c2f58c25 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/AuthCapabilityTypesTests.cs @@ -0,0 +1,103 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class AuthCapabilityTypesTests +{ + [Theory] + [InlineData(AcpProtocolVersion.V1, "", false)] + [InlineData(AcpProtocolVersion.V1, "\"terminal\":null", false)] + [InlineData(AcpProtocolVersion.V1, "\"terminal\":true", true)] + [InlineData(AcpProtocolVersion.V1, "\"terminal\":false", false)] + [InlineData(AcpProtocolVersion.V1, "\"terminal\":{}", false)] + [InlineData(AcpProtocolVersion.V1, "\"terminal\":42", false)] + [InlineData(AcpProtocolVersion.V2, "", false)] + [InlineData(AcpProtocolVersion.V2, "\"terminal\":null", false)] + [InlineData(AcpProtocolVersion.V2, "\"terminal\":true", false)] + [InlineData(AcpProtocolVersion.V2, "\"terminal\":false", false)] + [InlineData(AcpProtocolVersion.V2, "\"terminal\":{}", true)] + [InlineData(AcpProtocolVersion.V2, "\"terminal\":42", false)] + public void AuthCapabilities_Terminal_UsesOnlyTheNegotiatedWireShape(int version, string property, bool expected) + { + // Arrange + var json = $"{{{property}}}"; + + // Act + var direct = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + var parent = Assert.IsType(JsonSerializer.Deserialize($$"""{"auth":{{json}}}""", Wire.Of(version))); + + // Assert + Assert.Equal(expected, direct.Terminal); + Assert.Equal(expected, Assert.IsType(parent.Auth).Terminal); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "false")] + [InlineData(AcpProtocolVersion.V1, "42")] + [InlineData(AcpProtocolVersion.V1, "[]")] + [InlineData(AcpProtocolVersion.V2, "false")] + [InlineData(AcpProtocolVersion.V2, "42")] + [InlineData(AcpProtocolVersion.V2, "[]")] + public void AuthCapabilities_InvalidType_IsStrictAtRootAndDefaultableAsCapability(int version, string invalid) + { + // Arrange / Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(invalid, Wire.Of(version))); + var parent = Assert.IsType(JsonSerializer.Deserialize($$"""{"auth":{{invalid}}}""", Wire.Of(version))); + Assert.Null(parent.Auth); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, true, "{\"terminal\":true}")] + [InlineData(AcpProtocolVersion.V1, false, "{\"terminal\":false}")] + [InlineData(AcpProtocolVersion.V2, true, "{\"terminal\":{}}")] + [InlineData(AcpProtocolVersion.V2, false, "{}")] + public void AuthCapabilities_Writing_UsesOnlyTheNegotiatedWireShape(int version, bool terminal, string expected) + { + // Arrange + var capabilities = new AuthCapabilities { Terminal = terminal }; + + // Act + var json = JsonSerializer.Serialize(capabilities, Wire.Of(version)); + + // Assert + Assert.Equal(expected, json); + } + + [Fact] + public void AuthCapabilities_DefaultSourceGeneratedContract_RemainsStable() + { + // Arrange + var capabilities = new AuthCapabilities { Terminal = true }; + + // Act + var json = JsonSerializer.Serialize(capabilities, AcpJsonContext.Default.AuthCapabilities); + var restored = JsonSerializer.Deserialize(json, AcpJsonContext.Default.AuthCapabilities); + + // Assert + Assert.Equal("{\"terminal\":true}", json); + Assert.True(Assert.IsType(restored).Terminal); + Assert.Null(ClientCapabilityDefaults.Create().Auth); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1)] + [InlineData(AcpProtocolVersion.V2)] + public void AuthCapabilities_UnknownFields_SurviveKnownValueChanges(int version) + { + // Arrange + const string future = """{"number":1.20e+02,"text":"\u4f60"}"""; + var json = $$"""{"future":{{future}},"_meta":false}"""; + var parsed = Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))); + + // Act + using var replay = JsonDocument.Parse(JsonSerializer.Serialize(parsed with { Terminal = true }, Wire.Of(version))); + + // Assert + Assert.Null(parsed.Meta); + Assert.Equal(future, replay.RootElement.GetProperty("future").GetRawText()); + Assert.Equal(version == AcpProtocolVersion.V1 ? JsonValueKind.True : JsonValueKind.Object, + replay.RootElement.GetProperty("terminal").ValueKind); + } +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs new file mode 100644 index 000000000..6ba9c1ec1 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs @@ -0,0 +1,211 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class V2WireReviewBoundaryTests +{ + [Theory] + [InlineData(AcpProtocolVersion.V1, "direct")] + [InlineData(AcpProtocolVersion.V1, "list")] + [InlineData(AcpProtocolVersion.V1, "nested")] + [InlineData(AcpProtocolVersion.V2, "direct")] + [InlineData(AcpProtocolVersion.V2, "list")] + [InlineData(AcpProtocolVersion.V2, "nested")] + public void ConfigGroup_UnknownFields_SurviveWithoutOverwritingIdentity(int version, string root) + { + // Arrange + var groupId = version == AcpProtocolVersion.V1 ? "group" : "groupId"; + var unknownId = version == AcpProtocolVersion.V1 ? "groupId" : "group"; + var configId = version == AcpProtocolVersion.V1 ? "id" : "configId"; + const string future = """{"number":1.20e+02,"text":"\u4f60"}"""; + var json = $$"""{"{{groupId}}":"models","{{unknownId}}":"unrelated","name":"Models","future":{{future}},"options":[{"value":"fast","name":"Fast","future":{{future}}}]}"""; + + // Act + var replay = root switch + { + "direct" => RoundTrip(json, Wire.Of(version)), + "list" => RoundTrip($"[{json}]", Wire.Of>(version)), + _ => RoundTrip($$"""{"{{configId}}":"model","name":"Model","type":"select","currentValue":"fast","options":[{{json}}]}""", Wire.Of(version)) + }; + using var document = JsonDocument.Parse(replay); + var group = root == "direct" ? document.RootElement + : root == "list" ? document.RootElement[0] : document.RootElement.GetProperty("options")[0]; + + // Assert + Assert.Equal("models", group.GetProperty(groupId).GetString()); + Assert.Equal("unrelated", group.GetProperty(unknownId).GetString()); + Assert.Equal(future, group.GetProperty("future").GetRawText()); + Assert.Equal(future, group.GetProperty("options")[0].GetProperty("future").GetRawText()); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "direct")] + [InlineData(AcpProtocolVersion.V1, "list")] + [InlineData(AcpProtocolVersion.V1, "nested")] + [InlineData(AcpProtocolVersion.V2, "direct")] + [InlineData(AcpProtocolVersion.V2, "list")] + [InlineData(AcpProtocolVersion.V2, "nested")] + public void ConfigValue_UnknownFields_SurviveAtEveryRoot(int version, string root) + { + // Arrange + var configId = version == AcpProtocolVersion.V1 ? "id" : "configId"; + const string future = """{"number":1.20e+02,"text":"\u4f60"}"""; + var json = $$"""{"value":"fast","name":"Fast","future":{{future}}}"""; + + // Act + var replay = root switch + { + "direct" => RoundTrip(json, Wire.Of(version)), + "list" => RoundTrip($"[{json}]", Wire.Of>(version)), + _ => RoundTrip($$"""{"{{configId}}":"model","name":"Model","type":"select","currentValue":"fast","options":[{{json}}]}""", Wire.Of(version)) + }; + using var document = JsonDocument.Parse(replay); + var option = root == "direct" ? document.RootElement + : root == "list" ? document.RootElement[0] : document.RootElement.GetProperty("options")[0]; + + // Assert + Assert.Equal("fast", option.GetProperty("value").GetString()); + Assert.Equal(future, option.GetProperty("future").GetRawText()); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, true)] + [InlineData(AcpProtocolVersion.V1, false)] + [InlineData(AcpProtocolVersion.V2, true)] + [InlineData(AcpProtocolVersion.V2, false)] + public void ClientAuthCapability_ValidTerminal_SurvivesDirectAndInitialize(int version, bool direct) + { + // Arrange + var terminal = version == AcpProtocolVersion.V1 ? "true" : """{"_meta":{"source":"agent"},"future":1.20e+02}"""; + var capabilities = $$$$"""{"auth":{"terminal":{{{{terminal}}}}},"elicitation":{"form":{}}}"""; + var json = version == AcpProtocolVersion.V1 + ? $$"""{"protocolVersion":1,"clientInfo":{"name":"client","version":"1"},"clientCapabilities":{{capabilities}}}""" + : $$"""{"protocolVersion":2,"info":{"name":"client","version":"1"},"capabilities":{{capabilities}}}"""; + + // Act + var replay = direct ? RoundTrip(capabilities, Wire.Of(version)) + : RoundTrip(json, AcpJsonContext.Default.InitializeParams); + using var document = JsonDocument.Parse(replay); + var result = direct ? document.RootElement + : document.RootElement.GetProperty(version == AcpProtocolVersion.V1 ? "clientCapabilities" : "capabilities"); + + // Assert + Assert.Equal(terminal, result.GetProperty("auth").GetProperty("terminal").GetRawText()); + Assert.Equal(JsonValueKind.Object, result.GetProperty("elicitation").GetProperty("form").ValueKind); + } + + [Theory] + [InlineData("false", true)] + [InlineData("42", true)] + [InlineData("[]", true)] + [InlineData("false", false)] + [InlineData("42", false)] + [InlineData("[]", false)] + public void ClientCapabilitiesV2_InvalidElicitation_PreservesValidAuth(string invalid, bool direct) + { + // Arrange + var capabilities = $$$$"""{"elicitation":{{{{invalid}}}},"auth":{"terminal":{}},"_meta":{"peer":"client"}}"""; + + // Act + var replay = RoundTripClientCapabilities(capabilities, direct); + using var document = JsonDocument.Parse(replay); + var result = direct ? document.RootElement : document.RootElement.GetProperty("capabilities"); + + // Assert + Assert.False(result.TryGetProperty("elicitation", out _)); + Assert.Equal(JsonValueKind.Object, result.GetProperty("auth").GetProperty("terminal").ValueKind); + Assert.Equal("client", result.GetProperty("_meta").GetProperty("peer").GetString()); + } + + [Theory] + [InlineData("false", true)] + [InlineData("42", true)] + [InlineData("[]", true)] + [InlineData("false", false)] + [InlineData("42", false)] + [InlineData("[]", false)] + public void ClientCapabilitiesV2_InvalidAuth_PreservesValidElicitation(string invalid, bool direct) + { + // Arrange + var capabilities = $$$$"""{"auth":{{{{invalid}}}},"elicitation":{"form":{}},"_meta":{"peer":"client"}}"""; + + // Act + var replay = RoundTripClientCapabilities(capabilities, direct); + using var document = JsonDocument.Parse(replay); + var result = direct ? document.RootElement : document.RootElement.GetProperty("capabilities"); + + // Assert + Assert.False(result.TryGetProperty("auth", out _)); + Assert.Equal(JsonValueKind.Object, result.GetProperty("elicitation").GetProperty("form").ValueKind); + Assert.Equal("client", result.GetProperty("_meta").GetProperty("peer").GetString()); + } + + [Theory] + [InlineData("form", "url", true)] + [InlineData("url", "form", true)] + [InlineData("form", "url", false)] + [InlineData("url", "form", false)] + public void ElicitationV2_InvalidMarker_PreservesValidSibling(string invalid, string valid, bool direct) + { + // Arrange + var elicitation = $$$$"""{"{{{{invalid}}}}":false,"{{{{valid}}}}":{},"_meta":{"peer":"client"}}"""; + + // Act + var replay = direct ? RoundTrip(elicitation, Wire.V2()) + : RoundTripClientCapabilities($$"""{"elicitation":{{elicitation}}}""", false); + using var document = JsonDocument.Parse(replay); + var result = direct ? document.RootElement : document.RootElement.GetProperty("capabilities").GetProperty("elicitation"); + + // Assert + Assert.False(result.TryGetProperty(invalid, out _)); + Assert.Equal(JsonValueKind.Object, result.GetProperty(valid).ValueKind); + Assert.Equal("client", result.GetProperty("_meta").GetProperty("peer").GetString()); + } + + [Theory] + [InlineData("false", true)] + [InlineData("42", true)] + [InlineData("[]", true)] + [InlineData("false", false)] + [InlineData("42", false)] + [InlineData("[]", false)] + public void AgentCapabilitiesV2_InvalidAuth_PreservesValidSibling(string invalid, bool direct) + { + // Arrange + var capabilities = $$$$"""{"auth":{{{{invalid}}}},"session":{"prompt":{"image":{}}},"_meta":{"peer":"agent"}}"""; + + // Act + var replay = direct ? RoundTrip(capabilities, Wire.V2()) + : RoundTrip($$"""{"protocolVersion":2,"info":{"name":"agent","version":"1"},"capabilities":{{capabilities}}}""", AcpJsonContext.Default.InitializeResponse); + using var document = JsonDocument.Parse(replay); + var result = direct ? document.RootElement : document.RootElement.GetProperty("capabilities"); + + // Assert + Assert.False(result.TryGetProperty("auth", out _)); + Assert.Equal("agent", result.GetProperty("_meta").GetProperty("peer").GetString()); + if (!direct) + { + Assert.Equal(JsonValueKind.Object, result.GetProperty("session").GetProperty("prompt").GetProperty("image").ValueKind); + } + } + + [Theory] + [InlineData("false")] + [InlineData("42")] + [InlineData("[]")] + public void CapabilitiesV2_InvalidTypedRoot_RemainsStrict(string invalid) + { + // Arrange / Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(invalid, Wire.V2())); + Assert.Throws(() => JsonSerializer.Deserialize(invalid, Wire.V2())); + } + + private static string RoundTripClientCapabilities(string capabilities, bool direct) + => direct ? RoundTrip(capabilities, Wire.V2()) + : RoundTrip($$"""{"protocolVersion":2,"info":{"name":"client","version":"1"},"capabilities":{{capabilities}}}""", AcpJsonContext.Default.InitializeParams); + + private static string RoundTrip(string json, System.Text.Json.Serialization.Metadata.JsonTypeInfo typeInfo) + => JsonSerializer.Serialize(JsonSerializer.Deserialize(json, typeInfo), typeInfo); +} From 32323ba11dc0d67b5808304e7ca89cdfd13f0725 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Tue, 8 Sep 2026 21:01:47 +0000 Subject: [PATCH 3/6] docs(acp): describe completed draft wire contracts --- src/SalmonEgg.Acp/README.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/SalmonEgg.Acp/README.md b/src/SalmonEgg.Acp/README.md index 03dc784a2..eb33146a9 100644 --- a/src/SalmonEgg.Acp/README.md +++ b/src/SalmonEgg.Acp/README.md @@ -27,7 +27,7 @@ complete, and initializing a client with it throws. `AcpProtocolVersion.Latest` former name of `HighestModeled` and is kept only so 1.0.0 consumers still compile. Do not enable live v2 connections until prompt acknowledgement/state updates, versioned update -variants, permission subjects, config-option wire shapes, and JSON-RPC batches are implemented +variants, permission-subject handling, configuration workflows, and JSON-RPC batches are implemented and protected by a separate experimental feature flag. The modeled v2 contracts are marked `[Experimental("SEACP002")]`; see [ACP v2 draft surface](#acp-v2-draft-surface-seacp002). @@ -39,17 +39,19 @@ hosts must enable optional capabilities only after implementing their interactio | Surface | Current behavior | Remaining work | | --- | --- | --- | -| Agent authentication | An eligibility check blocks `terminal` and other non-blank unknown method types before `authenticate`. | Blank and malformed discriminators still need correction in [#147](https://github.com/salmonloop/salmon-egg/issues/147). Interactive terminal authentication also needs a host implementation before advertising `auth.terminal`. | +| Agent authentication | Only an absent discriminator or the exact `agent` type can reach `authenticate`. Unsupported strings round-trip without being selected; non-string discriminators are rejected. | Interactive terminal authentication still needs a host implementation before opting into `ClientCapabilities.Auth.Terminal`; see [#147](https://github.com/salmonloop/salmon-egg/issues/147). | | Request cancellation | The SDK implements `$/cancel_request`, `-32800`, and late-response correlation. `session/cancel` remains a separate session operation. | Network adapter cancellation and cancel-send error handling still need correction in [#148](https://github.com/salmonloop/salmon-egg/issues/148). Peer cancellation is best effort. | | Form elicitation | SalmonEgg's capability defaults advertise form mode. Hosts handle `ElicitationRequested` and return a typed accept, decline, or cancel response. | The host owns the form UI and must preserve the request's scope and connection ownership. | | URL elicitation | URL wire contracts and SDK completion tracking exist, but URL mode is not advertised by default. | A host must provide explicit navigation consent, a context the Agent cannot inspect, and a UI driven by the SDK's completion events. SalmonEgg's platform integration is tracked in [#154](https://github.com/salmonloop/salmon-egg/issues/154); [#146](https://github.com/salmonloop/salmon-egg/issues/146) tracks the complete elicitation delivery. | -| ACP v2 | Experimental wire contracts and version-specific serialization tests exist. Live initialization rejects v2. | Wire coverage and the runtime lifecycle remain incomplete; see [#149](https://github.com/salmonloop/salmon-egg/issues/149). | +| ACP v2 | Explicit v2 contexts model grouped configuration IDs, message IDs, resource-link icons, command inputs, and version-specific initialization/session/MCP shapes. Live initialization rejects v2. | Runtime state, projections, permission handling, and batch processing remain incomplete; see [#149](https://github.com/salmonloop/salmon-egg/issues/149). | -V2 wire coverage still needs grouped config-option identifiers (`groupId`), required message IDs -on chunks, resource-link icons, command-input discriminators, and the treatment of v1-only fields -and MCP variants. Its permission subject types are not connected to live request handling. -Completing these contracts does not complete message upserts, streaming tool and terminal -projections, or the acknowledgement-to-`state_update` completion lifecycle. +V2 wire coverage includes `configId`/`groupId`, required `messageId` values, text/custom command +inputs, and v1-only session fields and MCP variants. Unknown extension fields are preserved; +default-on-error and skip-invalid-item behavior applies only where the upstream schema permits it. +Resource-link icons are available through the experimental `ResourceLinkDraftExtensions` helper, +so constructing them requires an explicit draft opt-in. Permission subject types are still not +connected to live request handling. These contracts do not supply message upserts, streaming tool +and terminal projections, or the acknowledgement-to-`state_update` completion lifecycle. Keep the v1 runtime and public API compatible while these gaps are addressed. Enabling v2 needs both the upstream stabilization/Agent prerequisites and end-to-end verification of the complete @@ -60,9 +62,10 @@ lifecycle. Passing DTO tests or suppressing `SEACP002` does not satisfy that req Every v2 draft contract on the public surface carries `[Experimental("SEACP002")]`, so naming one is a **compile error** by default rather than a warning. That is deliberate: v2 is still an upstream draft, no live client negotiates it (`AcpProtocolVersion.RuntimeServed` is v1), and code built on -these types cannot reach a real Agent today. The 37 marked types are the `state_update` work-state +these types cannot reach a real Agent today. The 38 marked types are the `state_update` work-state family, the whole-message upsert updates, the terminal updates, streaming tool-call content, the -v2 `plan_update` envelope, permission subjects, the v2 capability markers, and the structured diff. +v2 `plan_update` envelope, permission subjects, the v2 capability markers, the structured diff, +and `ResourceLinkDraftExtensions` for resource-link icons. To evaluate them anyway, opt in explicitly: From 7c6193e89971eff3547a47e01a5e283b8ede7733 Mon Sep 17 00:00:00 2001 From: Shangxin Date: Thu, 10 Sep 2026 17:37:05 +0000 Subject: [PATCH 4/6] fix(acp): recover malformed MCP entries at their parent boundary Apply schema default-on-error and skip-invalid-items only on MCP parent arrays. Keep standalone decoding strict and preserve SDK rejection of hand-built null requests. Reuse the existing source-generated list recovery policy for V2 config options. --- .../Protocol/OtherSessionTypes.cs | 28 +++- src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs | 18 +- .../Serialization/AcpWireFormat.cs | 4 +- .../DefaultableProtocolJsonConverters.cs | 21 ++- .../Protocol/AuthMethodTypesTests.cs | 3 +- .../McpServerCollectionRecoveryTests.cs | 158 ++++++++++++++++++ .../Protocol/V2WireContractTests.cs | 5 +- .../Protocol/V2WireReviewBoundaryTests.cs | 2 +- 8 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 tests/SalmonEgg.Acp.Tests/Protocol/McpServerCollectionRecoveryTests.cs diff --git a/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs b/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs index 368cd7c4f..62c1dcfa6 100644 --- a/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/OtherSessionTypes.cs @@ -82,8 +82,10 @@ public SessionCancelParams(string sessionId) /// Request parameters for the Session/Load method. /// Loads the history of an existing session. /// - public sealed record SessionLoadParams : AcpProtocolObject + public sealed record SessionLoadParams : AcpProtocolObject, IJsonOnDeserialized { + private List _mcpServers = new(); + /// /// Session ID (required). /// @@ -101,7 +103,12 @@ public sealed record SessionLoadParams : AcpProtocolObject /// ACP session/load requires this field to always be an array; send [] even when there is no MCP server. /// [JsonPropertyName("mcpServers")] - public List McpServers { get; init; } = new List(); + [JsonConverter(typeof(DefaultableProtocolListJsonConverter))] + public List McpServers + { + get => _mcpServers; + init => _mcpServers = value; + } /// /// Additional working directories. When non-empty, requires the Agent to declare @@ -135,6 +142,9 @@ public SessionLoadParams( McpServers = mcpServers ?? new List(); AdditionalDirectories = additionalDirectories; } + + // Normalize the inbound schema default without changing validation of hand-built requests. + void IJsonOnDeserialized.OnDeserialized() => _mcpServers ??= new List(); } /// @@ -304,8 +314,10 @@ public override void Write( /// to replay history, while replayFrom: { type: "start" } requests a full history replay (the V2 /// alternative to session/load). /// - public sealed record SessionResumeParams : AcpProtocolObject + public sealed record SessionResumeParams : AcpProtocolObject, IJsonOnDeserialized { + private List _mcpServers = new(); + /// /// Session ID (required). /// @@ -323,7 +335,12 @@ public sealed record SessionResumeParams : AcpProtocolObject /// ACP session/resume requires this field to always be an array; send [] even when there is no MCP server. /// [JsonPropertyName("mcpServers")] - public List McpServers { get; init; } = new List(); + [JsonConverter(typeof(DefaultableProtocolListJsonConverter))] + public List McpServers + { + get => _mcpServers; + init => _mcpServers = value; + } /// /// Additional working directories. When non-empty, requires the Agent to declare @@ -367,6 +384,9 @@ public SessionResumeParams( AdditionalDirectories = additionalDirectories; ReplayFrom = replayFrom; } + + // Normalize the inbound schema default without changing validation of hand-built requests. + void IJsonOnDeserialized.OnDeserialized() => _mcpServers ??= new List(); } /// diff --git a/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs b/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs index 01d04a7cc..196a9c8bf 100644 --- a/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/SessionNewTypes.cs @@ -3,6 +3,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using SalmonEgg.Acp.Mcp; +using SalmonEgg.Acp.Serialization; namespace SalmonEgg.Acp.Protocol { @@ -10,8 +11,10 @@ namespace SalmonEgg.Acp.Protocol /// Request parameters for the Session/New method. /// Used to create a new session. /// - public sealed record SessionNewParams : AcpProtocolObject + public sealed record SessionNewParams : AcpProtocolObject, IJsonOnDeserialized { + private List _mcpServers = new(); + /// /// The working directory for the session (required). /// @@ -19,10 +22,15 @@ public sealed record SessionNewParams : AcpProtocolObject public string Cwd { get; init; } = string.Empty; /// - /// List of MCP server configurations (required; the protocol requires this value to be an array). + /// List of MCP server configurations. Schema-permitted missing or invalid input defaults to an empty array. /// [JsonPropertyName("mcpServers")] - public List McpServers { get; init; } = new List(); + [JsonConverter(typeof(DefaultableProtocolListJsonConverter))] + public List McpServers + { + get => _mcpServers; + init => _mcpServers = value; + } /// /// Additional working directories. When non-empty, the Agent is required to declare @@ -53,6 +61,10 @@ public SessionNewParams( McpServers = mcpServers ?? new List(); AdditionalDirectories = additionalDirectories; } + + // Generated init-only setters can replace a missing field's initializer with null. + // Normalize the inbound default here; explicitly invalid API arguments still fail validation. + void IJsonOnDeserialized.OnDeserialized() => _mcpServers ??= new List(); } /// diff --git a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs index 03f48d65b..93841cc27 100644 --- a/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs +++ b/src/SalmonEgg.Acp/Serialization/AcpWireFormat.cs @@ -131,14 +131,14 @@ private void ApplyObjectContract(JsonTypeInfo info) if (info.Type == typeof(SessionNewResponse) || info.Type == typeof(SessionResumeResponse)) { IgnoreProperty(info, "modes", new IgnoredProtocolPropertyJsonConverter()); - FindProperty(info, "configOptions").CustomConverter = new DefaultableConfigOptionsJsonConverter(); + FindProperty(info, "configOptions").CustomConverter = new DefaultableProtocolListJsonConverter(); info.OnDeserialized = static value => NormalizeConfigOptions(value); } else if (info.Type == typeof(ConfigOptionUpdate) || info.Type == typeof(SessionSetConfigOptionResponse)) { var configOptions = FindProperty(info, "configOptions"); configOptions.IsRequired = true; - configOptions.CustomConverter = new DefaultableConfigOptionsJsonConverter(); + configOptions.CustomConverter = new DefaultableProtocolListJsonConverter(); info.OnSerializing = static value => RequireConfigOptions(value); } else if (info.Type == typeof(AgentAuthCapabilities)) diff --git a/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs index f158c1e34..9483fae6e 100644 --- a/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs +++ b/src/SalmonEgg.Acp/Serialization/DefaultableProtocolJsonConverters.cs @@ -62,22 +62,27 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions => JsonSerializer.Serialize(writer, value, (JsonTypeInfo)options.GetTypeInfo(typeof(T))); } -internal sealed class DefaultableConfigOptionsJsonConverter : JsonConverter> +internal sealed class DefaultableProtocolListJsonConverter : JsonConverter> where T : class { public override bool HandleNull => true; - public override List Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + public override List Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { using var document = JsonDocument.ParseValue(ref reader); - var result = new List(); + var result = new List(); if (document.RootElement.ValueKind != JsonValueKind.Array) { return result; } - var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ConfigOption)); + var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(T)); foreach (var item in document.RootElement.EnumerateArray()) { + if (item.ValueKind != JsonValueKind.Object) + { + continue; + } + try { if (item.Deserialize(typeInfo) is { } option) @@ -87,20 +92,20 @@ public override List Read(ref Utf8JsonReader reader, Type typeToCo } catch (JsonException) { - // Only configOptions has this pair of explicit schema recovery annotations: - // x-deserialize-default-on-error and x-deserialize-skip-invalid-items. + // Attach only to object-array properties with both schema recovery annotations: + // default-on-error and skip-invalid-items. Standalone roots remain strict. } } return result; } - public override void Write(Utf8JsonWriter writer, List value, JsonSerializerOptions options) + public override void Write(Utf8JsonWriter writer, List value, JsonSerializerOptions options) { writer.WriteStartArray(); if (value is not null) { - var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(ConfigOption)); + var typeInfo = (JsonTypeInfo)options.GetTypeInfo(typeof(T)); foreach (var option in value) { JsonSerializer.Serialize(writer, option, typeInfo); diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs index 245b71431..135251f35 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/AuthMethodTypesTests.cs @@ -186,7 +186,8 @@ public void DeserializeInitialize_InvalidAuthItems_PreservesValidAndUnknownMetho // Assert Assert.NotNull(response); - Assert.Equal(["good", "future"], response.AuthMethods!.Select(method => method.Id)); + Assert.NotNull(response.AuthMethods); + Assert.Equal(["good", "future"], response.AuthMethods.Select(method => method.Id)); Assert.False(response.AuthMethods[1].SupportsAuthenticateRequest); using var document = JsonDocument.Parse(replay); Assert.True(document.RootElement.GetProperty("authMethods")[1].GetProperty("vendor").GetProperty("preserve").GetBoolean()); diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/McpServerCollectionRecoveryTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/McpServerCollectionRecoveryTests.cs new file mode 100644 index 000000000..0d43628d0 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/McpServerCollectionRecoveryTests.cs @@ -0,0 +1,158 @@ +using System.Text.Json; +using SalmonEgg.Acp.Mcp; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; + +namespace SalmonEgg.Acp.Tests.Protocol; + +public sealed class McpServerCollectionRecoveryTests +{ + public static TheoryData RequestContracts => new() + { + { AcpProtocolVersion.V1, "new" }, + { AcpProtocolVersion.V1, "load" }, + { AcpProtocolVersion.V1, "resume" }, + { AcpProtocolVersion.V2, "new" }, + { AcpProtocolVersion.V2, "resume" } + }; + + [Theory] + [MemberData(nameof(RequestContracts))] + public void Deserialize_InvalidItems_PreservesValidServersAndUnknownPayloads(int version, string method) + { + // Arrange + const string future = """{"type":"vendor_pipe","name":"future","nested":{"number":1.20e+02,"text":"\u4f60"}}"""; + var json = $$""" + {"sessionId":"session","cwd":"/tmp","mcpServers":[ + 42,null,[],{}, + {"type":null,"name":"bad","command":"mcp"}, + {"type":"stdio","name":"bad-args","command":"mcp","args":42}, + {"type":"stdio","name":"good","command":"mcp","args":["serve"]}, + {{future}}, + {"type":"http","name":"http","url":"https://example.test/mcp","headers":[]} + ]} + """; + + // Act + var servers = ReadServers(json, version, method); + using var replay = JsonDocument.Parse(RoundTrip(json, version, method)); + + // Assert + Assert.Equal(["good", "future", "http"], servers.Select(server => server.Name)); + Assert.Equal(["serve"], Assert.IsType(servers[0]).Args); + Assert.IsType(servers[1]); + Assert.IsType(servers[2]); + Assert.Equal(future, replay.RootElement.GetProperty("mcpServers")[1].GetRawText()); + } + + [Theory] + [MemberData(nameof(RequestContracts))] + public void Deserialize_InvalidCollection_UsesEmptyDefault(int version, string method) + { + // Arrange + string[] invalidValues = ["null", "42", "true", "\"invalid\"", "{}"]; + + foreach (var rawValue in invalidValues) + { + var json = $$"""{"sessionId":"session","cwd":"/tmp","mcpServers":{{rawValue}}}"""; + + // Act + var servers = ReadServers(json, version, method); + + // Assert + Assert.Empty(servers); + } + } + + [Theory] + [InlineData(AcpProtocolVersion.V1, "resume")] + [InlineData(AcpProtocolVersion.V2, "new")] + [InlineData(AcpProtocolVersion.V2, "resume")] + public void Deserialize_OmittedOptionalCollection_UsesEmptyDefault(int version, string method) + { + // Arrange + const string json = """{"sessionId":"session","cwd":"/tmp"}"""; + + // Act + var servers = ReadServers(json, version, method); + + // Assert + Assert.Empty(servers); + } + + [Theory] + [InlineData("new")] + [InlineData("resume")] + public void Deserialize_MissingDiscriminator_UsesOnlyTheNegotiatedDefault(string method) + { + // Arrange + const string json = """ + {"sessionId":"session","cwd":"/tmp","mcpServers":[ + {"name":"legacy","command":"mcp"}, + {"type":"stdio","name":"explicit","command":"mcp"} + ]} + """; + + // Act + var stableServers = ReadServers(json, AcpProtocolVersion.V1, method); + var draftServers = ReadServers(json, AcpProtocolVersion.V2, method); + + // Assert + Assert.Equal(["legacy", "explicit"], stableServers.Select(server => server.Name)); + Assert.Equal("explicit", Assert.Single(draftServers).Name); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1)] + [InlineData(AcpProtocolVersion.V2)] + public void Deserialize_StandaloneMalformedServerOrList_RemainsStrict(int version) + { + // Arrange + const string json = """{"type":42,"name":"bad","command":"mcp"}"""; + + // Act / Assert + Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.Of(version))); + Assert.Throws(() => JsonSerializer.Deserialize($"[{json}]", Wire.Of>(version))); + } + + [Theory] + [InlineData("new")] + [InlineData("load")] + [InlineData("resume")] + public void Deserialize_DefaultGeneratedContext_UsesStableCollectionRecovery(string method) + { + // Arrange + const string json = """ + {"sessionId":"session","cwd":"/tmp","mcpServers":[42,{"name":"good","command":"mcp"}]} + """; + + // Act + var replayJson = method switch + { + "new" => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, AcpJsonContext.Default.SessionNewParams)), AcpJsonContext.Default.SessionNewParams), + "load" => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, AcpJsonContext.Default.SessionLoadParams)), AcpJsonContext.Default.SessionLoadParams), + _ => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, AcpJsonContext.Default.SessionResumeParams)), AcpJsonContext.Default.SessionResumeParams) + }; + using var replay = JsonDocument.Parse(replayJson); + + // Assert + Assert.Equal("good", Assert.Single(replay.RootElement.GetProperty("mcpServers").EnumerateArray()).GetProperty("name").GetString()); + Assert.False(replay.RootElement.GetProperty("mcpServers")[0].TryGetProperty("type", out _)); + } + + private static List ReadServers(string json, int version, string method) => method switch + { + "new" => JsonSerializer.Deserialize(json, Wire.Of(version))!.McpServers, + "load" => JsonSerializer.Deserialize(json, Wire.Of(version))!.McpServers, + "resume" => JsonSerializer.Deserialize(json, Wire.Of(version))!.McpServers, + _ => throw new ArgumentOutOfRangeException(nameof(method)) + }; + + private static string RoundTrip(string json, int version, string method) => method switch + { + "new" => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))), Wire.Of(version)), + "load" => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))), Wire.Of(version)), + "resume" => JsonSerializer.Serialize(Assert.IsType(JsonSerializer.Deserialize(json, Wire.Of(version))), Wire.Of(version)), + _ => throw new ArgumentOutOfRangeException(nameof(method)) + }; +} diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs index c7b377e92..a1aae03f3 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireContractTests.cs @@ -228,8 +228,9 @@ public void McpServerV2_MissingOrWrongDiscriminator_RejectsInsteadOfDefaultingTo // Act / Assert Assert.Throws(() => JsonSerializer.Deserialize(json, Wire.V2())); - Assert.Throws(() => JsonSerializer.Deserialize( - $$"""{"cwd":"/tmp","mcpServers":[{{json}}]}""", Wire.V2())); + var request = JsonSerializer.Deserialize( + $$"""{"cwd":"/tmp","mcpServers":[{{json}}]}""", Wire.V2()); + Assert.Empty(request!.McpServers); } [Fact] diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs index 6ba9c1ec1..e378fdf2d 100644 --- a/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Protocol/V2WireReviewBoundaryTests.cs @@ -207,5 +207,5 @@ private static string RoundTripClientCapabilities(string capabilities, bool dire : RoundTrip($$"""{"protocolVersion":2,"info":{"name":"client","version":"1"},"capabilities":{{capabilities}}}""", AcpJsonContext.Default.InitializeParams); private static string RoundTrip(string json, System.Text.Json.Serialization.Metadata.JsonTypeInfo typeInfo) - => JsonSerializer.Serialize(JsonSerializer.Deserialize(json, typeInfo), typeInfo); + => JsonSerializer.Serialize(JsonSerializer.Deserialize(json, typeInfo)!, typeInfo); } From cacbd9ae79edb982b125fbb23ddc0f5dc3a65b1e Mon Sep 17 00:00:00 2001 From: Shangxin Date: Thu, 10 Sep 2026 20:30:43 +0000 Subject: [PATCH 5/6] fix(chat): keep draft projections aligned with the latest refresh Read committed connection state inside one projection owner and reject older callbacks after asynchronous reads, clear or disposal. Exercise same-connection, replacement, clear and disposal races through the existing store and UI dispatcher seams. --- .../Chat/ChatViewModel.NewSessionDraft.cs | 62 ++++--- .../ViewModels/Chat/ChatViewModel.cs | 7 +- ...ChatViewModelTests.NewSessionProjection.cs | 154 ++++++++++++++++++ .../Chat/ChatViewModelTests.cs | 22 +-- 4 files changed, 189 insertions(+), 56 deletions(-) create mode 100644 tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.NewSessionProjection.cs diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.NewSessionDraft.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.NewSessionDraft.cs index 69ff65b63..539789c00 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.NewSessionDraft.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.NewSessionDraft.cs @@ -142,8 +142,7 @@ await PublishRequiredProfileConnectionFailureDraftAsync( Error = cwdResolution.ErrorMessage ?? AcpSessionNewCwdResolver.MissingRemoteCwdMessage }; await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(failed)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } @@ -158,7 +157,7 @@ await ApplyNewSessionDraftProjectionAsync( var existingDraft = connectionState.NewSessionDraft; if (IsReusableNewSessionDraft(existingDraft, profileId!, connectionInstanceId!, normalizedCwd)) { - await ApplyNewSessionDraftProjectionAsync(connectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } @@ -189,8 +188,7 @@ await ApplyNewSessionDraftProjectionAsync( AcpSessionUpdateDelta.Empty, isConfigAuthoritative: false); await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(creatingDraft)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); var request = new PendingNewSessionDraftRequest( requestKey, @@ -219,8 +217,7 @@ await ApplyNewSessionDraftProjectionAsync( Error = ex.Message }; await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(failed)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } finally { @@ -364,7 +361,7 @@ public async Task DiscardNewSessionDraftAsync(CancellationToken cancellationToke var draft = connectionState.NewSessionDraft; if (draft is null) { - await ApplyNewSessionDraftProjectionAsync(connectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } @@ -451,10 +448,6 @@ await _chatStore.Dispatch(new MergeConversationSessionStateAction( } } - internal async Task ApplyLatestNewSessionDraftProjectionAsync() - => await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); - private void QueueNewSessionDraftModeSelection(SessionModeViewModel? mode) { try @@ -529,7 +522,7 @@ private async Task SetNewSessionDraftModeAsync(string modeId, CancellationToken new SessionSetConfigOptionParams(draft.RemoteSessionId!, modeConfigId!, modeId)).ConfigureAwait(false); if (response.ConfigOptions is null) { - await ApplyNewSessionDraftProjectionAsync(connectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } @@ -551,8 +544,7 @@ private async Task SetNewSessionDraftModeAsync(string modeId, CancellationToken var updatedDraft = MergeNewSessionDraftDelta(draft, delta); await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(updatedDraft)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { @@ -560,8 +552,7 @@ await ApplyNewSessionDraftProjectionAsync( catch (Exception ex) { Logger.LogWarning(ex, "Failed to switch ACP new-session draft mode."); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } finally { @@ -603,7 +594,7 @@ private async Task SetNewSessionDraftModelAsync(string modelValue, CancellationT new SessionSetConfigOptionParams(draft.RemoteSessionId!, modelConfigId!, modelValue)).ConfigureAwait(false); if (response.ConfigOptions is null) { - await ApplyNewSessionDraftProjectionAsync(connectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } @@ -616,8 +607,7 @@ private async Task SetNewSessionDraftModelAsync(string modelValue, CancellationT var updatedDraft = MergeNewSessionDraftDelta(draft, delta); await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(updatedDraft)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { @@ -625,8 +615,7 @@ await ApplyNewSessionDraftProjectionAsync( catch (Exception ex) { Logger.LogWarning(ex, "Failed to switch ACP new-session draft model."); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } finally { @@ -634,13 +623,22 @@ await ApplyNewSessionDraftProjectionAsync( } } - private async Task ApplyNewSessionDraftProjectionAsync(ChatConnectionState connectionState) + internal async Task ApplyLatestNewSessionDraftProjectionAsync() { + var projectionVersion = Interlocked.Increment(ref _newSessionDraftProjectionVersion); + var connectionState = await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false); var draft = ResolveEffectiveNewSessionDraft(connectionState); var storeState = await _chatStore.GetCurrentStateAsync().ConfigureAwait(false); var connectionProjection = CreateProjection(storeState, connectionState); await PostToUiAsync(() => { + // Store subscriptions and command completions can finish out of order. Only the + // latest refresh may project a snapshot, and clearing a draft invalidates queued work. + if (_disposed || projectionVersion != Volatile.Read(ref _newSessionDraftProjectionVersion)) + { + return; + } + ApplyConversationStatusProjection(connectionProjection); ApplyConnectionAndAgentProjection(connectionProjection); @@ -688,6 +686,7 @@ await PostToUiAsync(() => private void ClearNewSessionDraftProjection() { + Interlocked.Increment(ref _newSessionDraftProjectionVersion); IsNewSessionDraftLoading = false; IsNewSessionDraftReady = false; NewSessionDraftErrorMessage = string.Empty; @@ -720,13 +719,12 @@ private async Task ClearNewSessionDraftStateAsync(bool clearDesiredRequest = tru var current = await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false); if (current.NewSessionDraft is null) { - await ApplyNewSessionDraftProjectionAsync(current).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); return; } await _chatConnectionStore.Dispatch(new ClearNewSessionDraftAction()).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } private async Task PublishRequiredProfileConnectionFailureDraftAsync( @@ -755,8 +753,7 @@ private async Task PublishRequiredProfileConnectionFailureDraftAsync( : errorMessage }; await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(failed)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } private void SetSelectedNewSessionDraftModeWithoutDispatch(SessionModeViewModel? mode) @@ -1077,8 +1074,7 @@ private async Task CompleteSuccessfulNewSessionDraftRequestAsync( shouldDiscardResponse = true; ClearDesiredNewSessionDraftRequestKey(); await _chatConnectionStore.Dispatch(new ClearNewSessionDraftAction()).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } else if (!ShouldAdoptNewSessionDraftRequestResponse(connectionState, request.RequestKey)) { @@ -1096,8 +1092,7 @@ await ApplyNewSessionDraftProjectionAsync( _acpSessionUpdateProjector.ProjectSessionNew(response), response.ConfigOptions is not null); await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(readyDraft)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); Logger.LogInformation( "Applied ACP new-session draft response. profileId={ProfileId} connectionInstanceId={ConnectionInstanceId} remoteSessionId={RemoteSessionId} modeCount={ModeCount}", request.ProfileId, @@ -1155,8 +1150,7 @@ private async Task CompleteFailedNewSessionDraftRequestAsync( Error = exception.Message }; await _chatConnectionStore.Dispatch(new SetNewSessionDraftAction(failed)).ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync( - await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false)).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); appliedFailure = true; } } diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs index 3dbb86e0f..5e80410b6 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs @@ -156,6 +156,7 @@ private enum HydrationOverlayPhase private readonly ObservableCollection _newSessionDraftModeOptions = new(); private readonly ObservableCollection _newSessionDraftModelOptions = new(); private readonly SemaphoreSlim _newSessionDraftGate = new(1, 1); + private long _newSessionDraftProjectionVersion; private IChatService? _chatService; private IReadOnlyList _currentMcpServers = Array.Empty(); private IReadOnlyList _modelOptions = Array.Empty(); @@ -1613,8 +1614,7 @@ private void StartStoreProjection() } await RefreshProjectionAsync(state, token, ct).ConfigureAwait(false); - var latestConnectionState = await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync(latestConnectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); }, out _connectionStateSubscription); } @@ -1741,8 +1741,7 @@ private async void OnLanguageChanged(object? sender, EventArgs e) // New-session draft lives on the connection store and is not part of the chat-store // projection. Re-apply it so held draft fault messages re-run NormalizeNewSessionDraftError // against the current language (except English identity sentinels). - var connectionState = await _chatConnectionStore.GetCurrentStateAsync().ConfigureAwait(false); - await ApplyNewSessionDraftProjectionAsync(connectionState).ConfigureAwait(false); + await ApplyLatestNewSessionDraftProjectionAsync().ConfigureAwait(false); } catch (OperationCanceledException) { diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.NewSessionProjection.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.NewSessionProjection.cs new file mode 100644 index 000000000..d9f41e99d --- /dev/null +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.NewSessionProjection.cs @@ -0,0 +1,154 @@ +using System.Collections.Immutable; +using SalmonEgg.Domain.Models.Conversation; +using SalmonEgg.Presentation.Core.Mvux.Chat; + +namespace SalmonEgg.Presentation.Core.Tests.Chat; + +public partial class ChatViewModelTests +{ + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NewSessionDraftProjection_PendingRead_CannotRestoreAfterClearOrDisposal(bool dispose) + { + // Arrange + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateViewModel(dispatcher); + var ready = new NewSessionDraftState( + "profile", "/work", "session", "connection", NewSessionDraftPhase.Ready, 1, + ImmutableList.Create(new ConversationModeOptionSnapshot { ModeId = "code", ModeName = "Code" }), "code", + ImmutableList.Empty, false, + ImmutableList.Empty, null); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetNewSessionDraftAction(ready)).AsTask()); + await fixture.ApplyNewSessionDraftProjectionAsync(); + await dispatcher.RunUntilIdleAsync(); + var blocked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + fixture.ChatStore.ReadState = async () => + { + fixture.ChatStore.ReadState = null; + blocked.TrySetResult(); + await release.Task.WaitAsync(TestContext.Current.CancellationToken); + return fixture.ChatStore.LatestState; + }; + var oldProjection = fixture.ViewModel.ApplyLatestNewSessionDraftProjectionAsync(); + try + { + await blocked.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + var changed = 0; + if (dispose) + { + fixture.ViewModel.Dispose(); + fixture.ViewModel.PropertyChanged += (_, _) => changed++; + } + else + { + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new ClearNewSessionDraftAction()).AsTask()); + await fixture.ApplyNewSessionDraftProjectionAsync(); + } + + // Act + release.TrySetResult(); + await AwaitWithSynchronizationContextAsync(dispatcher, oldProjection); + await dispatcher.RunUntilIdleAsync(); + + // Assert + if (dispose) + { + Assert.Equal(0, changed); + } + else + { + Assert.False(fixture.ViewModel.IsNewSessionDraftReady); + Assert.Empty(fixture.ViewModel.NewSessionDraftModeOptions); + } + } + finally + { + fixture.ChatStore.ReadState = null; + release.TrySetResult(); + await AwaitWithSynchronizationContextAsync(dispatcher, oldProjection); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NewSessionDraftProjection_DelayedOldState_CannotReplaceNewReadyState(bool replaceConnection) + { + // Arrange + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateViewModel(dispatcher); + var emptyDraft = new NewSessionDraftState( + "profile", "/work", null, "connection", NewSessionDraftPhase.Creating, 1, + ImmutableList.Empty, null, + ImmutableList.Empty, false, + ImmutableList.Empty, null); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetSelectedProfileIntentAction("profile")).AsTask()); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetForegroundTransportProfileAction("profile")).AsTask()); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetConnectionInstanceIdAction("connection")).AsTask()); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetConnectionPhaseAction(ConnectionPhase.Connected)).AsTask()); + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetNewSessionDraftAction(emptyDraft)).AsTask()); + await fixture.ApplyNewSessionDraftProjectionAsync(); + await dispatcher.RunUntilIdleAsync(); + + var blocked = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + fixture.ChatStore.ReadState = async () => + { + fixture.ChatStore.ReadState = null; + blocked.TrySetResult(); + await release.Task.WaitAsync(TestContext.Current.CancellationToken); + return fixture.ChatStore.LatestState; + }; + var oldProjection = fixture.ViewModel.ApplyLatestNewSessionDraftProjectionAsync(); + try + { + await blocked.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + var connection = replaceConnection ? "replacement" : "connection"; + if (replaceConnection) + { + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetConnectionInstanceIdAction(connection)).AsTask()); + } + var ready = emptyDraft with + { + ConnectionInstanceId = connection, + Phase = NewSessionDraftPhase.Ready, + Version = 2, + RemoteSessionId = "ready-session", + AvailableModes = ImmutableList.Create(new ConversationModeOptionSnapshot { ModeId = "code", ModeName = "Code" }), + SelectedModeId = "code" + }; + await AwaitWithSynchronizationContextAsync(dispatcher, + fixture.DispatchConnectionAsync(new SetNewSessionDraftAction(ready)).AsTask()); + await fixture.ApplyNewSessionDraftProjectionAsync(); + await dispatcher.RunUntilIdleAsync(); + Assert.True(fixture.ViewModel.IsNewSessionDraftReady); + + // Act: let the old projection finish only after the latest state is visible. + release.TrySetResult(); + await AwaitWithSynchronizationContextAsync(dispatcher, oldProjection); + await dispatcher.RunUntilIdleAsync(); + + // Assert + Assert.True(fixture.ViewModel.IsNewSessionDraftReady); + Assert.False(fixture.ViewModel.IsNewSessionDraftLoading); + Assert.Equal("code", Assert.Single(fixture.ViewModel.NewSessionDraftModeOptions).ModeId); + Assert.Equal("code", fixture.ViewModel.SelectedNewSessionDraftMode?.ModeId); + } + finally + { + fixture.ChatStore.ReadState = null; + release.TrySetResult(); + await AwaitWithSynchronizationContextAsync(dispatcher, oldProjection); + } + } +} diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.cs index 03ddd76cd..b421e16a0 100644 --- a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.cs +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.cs @@ -6629,18 +6629,6 @@ private static Task ApplyCurrentStoreProjectionAsync(ChatViewModel viewModel) return (Task)method!.Invoke(viewModel, [null])!; } - private static Task ApplyNewSessionDraftProjectionAsync(ChatViewModel viewModel, ChatConnectionState connectionState) - { - var method = typeof(ChatViewModel).GetMethod( - "ApplyNewSessionDraftProjectionAsync", - BindingFlags.Instance | BindingFlags.NonPublic, - binder: null, - types: [typeof(ChatConnectionState)], - modifiers: null); - Assert.NotNull(method); - return (Task)method!.Invoke(viewModel, [connectionState])!; - } - private static JsonElement ParseJsonParams(string json) { using var document = JsonDocument.Parse(json); @@ -7365,16 +7353,13 @@ public async Task ApplyNewSessionDraftProjectionAsync() if (_uiDispatcher is QueueingSynchronizationContext queuedDispatcher) { await queuedDispatcher.RunUntilIdleAsync(); - var latestConnectionState = await GetConnectionStateAsync(); await queuedDispatcher.RunUntilCompletedAsync( - ChatViewModelTests.ApplyNewSessionDraftProjectionAsync(ViewModel, latestConnectionState)); + ViewModel.ApplyLatestNewSessionDraftProjectionAsync()); await queuedDispatcher.RunUntilIdleAsync(); return; } - var connectionState = await GetConnectionStateAsync(); - var projectionTask = ChatViewModelTests.ApplyNewSessionDraftProjectionAsync(ViewModel, connectionState); - await projectionTask; + await ViewModel.ApplyLatestNewSessionDraftProjectionAsync(); } public async ValueTask DisposeAsync() @@ -7585,6 +7570,7 @@ public RecordingChatStore(IState state, IWorkspaceWriter? workspaceWr public IState State { get; } public ChatState LatestState { get; private set; } public Func? AfterDispatch { get; set; } + public Func>? ReadState { get; set; } public IReadOnlyCollection Actions => _actions.ToArray(); @@ -7623,7 +7609,7 @@ public async ValueTask SetStateAsync(ChatState state) } public ValueTask GetCurrentStateAsync() - => ValueTask.FromResult(LatestState); + => ReadState?.Invoke() ?? ValueTask.FromResult(LatestState); } [Fact] From ef7245b17ca4d3ebb0fc0683b37569e8c20d9c3b Mon Sep 17 00:00:00 2001 From: Shangxin Date: Thu, 10 Sep 2026 20:30:43 +0000 Subject: [PATCH 6/6] test(chat): synchronize explicit connection and concurrency fixtures Use non-connecting profile selection when a fixture supplies connection state, preventing unconfigured automatic connections from overwriting it. Rendezvous two snapshot workers for bounded iterations instead of cancelling queued workers before they run. --- .../Chat/ChatConversationWorkspaceTests.cs | 63 +++++++++---------- .../Start/StartViewModelTests.cs | 8 ++- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatConversationWorkspaceTests.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatConversationWorkspaceTests.cs index ad7abe7b8..ff43ddc7b 100644 --- a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatConversationWorkspaceTests.cs +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatConversationWorkspaceTests.cs @@ -444,64 +444,59 @@ public async Task GetConversationSnapshot_WhenTranscriptMutatesConcurrently_Does CreatedAt: new DateTime(2026, 3, 1, 0, 0, 0, DateTimeKind.Utc), LastUpdatedAt: new DateTime(2026, 3, 1, 0, 1, 0, DateTimeKind.Utc))); - var started = new ManualResetEventSlim(false); - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(1)); - Exception? failure = null; - - var snapshotReader = Task.Run(() => + const int iterations = 32; + using var start = new Barrier(2); + using var lifetime = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var snapshotsRead = 0; + var transcriptsWritten = 0; + + // Dedicated workers rendezvous before every operation. A short timer can expire before + // a queued worker starts under CI load and would test scheduling instead of concurrent access. + var snapshotReader = Task.Factory.StartNew(() => { - started.Wait(cts.Token); try { - while (!cts.IsCancellationRequested) + for (var i = 0; i < iterations; i++) { - _ = workspace.GetConversationSnapshot("session-1"); + start.SignalAndWait(lifetime.Token); + Assert.NotNull(workspace.GetConversationSnapshot("session-1")); + snapshotsRead++; } } - catch (OperationCanceledException) when (cts.IsCancellationRequested) - { - } - catch (Exception ex) + catch { - failure = ex; - cts.Cancel(); + lifetime.Cancel(); + throw; } - }, cancellationToken: TestContext.Current.CancellationToken); + }, TestContext.Current.CancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default); - var transcriptMutator = Task.Run(() => + var transcriptMutator = Task.Factory.StartNew(() => { - started.Wait(cts.Token); try { - var counter = 0; - while (!cts.IsCancellationRequested) + for (var i = 0; i < iterations; i++) { + start.SignalAndWait(lifetime.Token); workspace.UpsertConversationSnapshot(new ConversationWorkspaceSnapshot( ConversationId: "session-1", - Transcript: CreateTranscript($"mutated-{counter}", 1024), + Transcript: CreateTranscript($"mutated-{i}", 1024), Plan: Array.Empty(), ShowPlanPanel: false, CreatedAt: new DateTime(2026, 3, 1, 0, 0, 0, DateTimeKind.Utc), - LastUpdatedAt: new DateTime(2026, 3, 1, 0, 1, 0, DateTimeKind.Utc).AddSeconds(counter))); - counter++; + LastUpdatedAt: new DateTime(2026, 3, 1, 0, 1, 0, DateTimeKind.Utc).AddSeconds(i))); + transcriptsWritten++; } } - catch (OperationCanceledException) when (cts.IsCancellationRequested) + catch { + lifetime.Cancel(); + throw; } - catch (Exception ex) - { - failure = ex; - cts.Cancel(); - } - }, cancellationToken: TestContext.Current.CancellationToken); + }, TestContext.Current.CancellationToken, TaskCreationOptions.LongRunning, TaskScheduler.Default); - started.Set(); - await Task.Delay(200, CancellationToken.None); - cts.Cancel(); await Task.WhenAll(snapshotReader, transcriptMutator); - - Assert.Null(failure); + Assert.Equal(iterations, snapshotsRead); + Assert.Equal(iterations, transcriptsWritten); } [Fact] diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Start/StartViewModelTests.cs b/tests/SalmonEgg.Presentation.Core.Tests/Start/StartViewModelTests.cs index b61fd5844..30402fdca 100644 --- a/tests/SalmonEgg.Presentation.Core.Tests/Start/StartViewModelTests.cs +++ b/tests/SalmonEgg.Presentation.Core.Tests/Start/StartViewModelTests.cs @@ -1196,7 +1196,7 @@ public async Task StartModeSelector_WhenSelectedAgentSwitches_WaitsForForeground StdioCommand = "acp-two", ConnectionTimeout = 10 }); - chat.ViewModel.SelectedAcpProfile = chat.ViewModel.AcpProfileList[0]; + await chat.ViewModel.SelectProfileAsync(chat.ViewModel.AcpProfileList[0], TestContext.Current.CancellationToken); var chatService = CreateConnectedChatService(); chatService.SetupGet(service => service.AgentCapabilities) @@ -1246,7 +1246,7 @@ await WaitForConditionAsync( && startViewModel.StartModeOptions.Count == 2, timeoutMilliseconds: 10000); - chat.ViewModel.SelectedAcpProfile = chat.ViewModel.AcpProfileList[1]; + await chat.ViewModel.SelectProfileAsync(chat.ViewModel.AcpProfileList[1], TestContext.Current.CancellationToken); await chat.DispatchConnectionAsync(new SetConnectionPhaseAction(ConnectionPhase.Connecting)); await chat.DispatchConnectionAsync(new SetConnectionInstanceIdAction("conn-2")); await chat.DispatchConnectionAsync(new SetForegroundTransportProfileAction("profile-2")); @@ -3661,7 +3661,9 @@ private static void SelectStdioProfile(ChatViewModelHarness chat, string profile Transport = TransportType.Stdio, StdioCommand = "agent" }); - chat.ViewModel.SelectedAcpProfile = chat.ViewModel.AcpProfileList[^1]; + // These launch tests publish connection/draft facts explicitly. Profile selection must + // not race them with the unrelated automatic connection command's unconfigured mock. + chat.ViewModel.SelectProfileForDefaultProjection(chat.ViewModel.AcpProfileList[^1]); } private static async Task MakeStartDraftReadyAsync(