fix(acp): route derived declared roots through the McpServer converter - #231
Merged
Merged
Conversation
The v1-only sse write gate rides on the base McpServer [JsonConverter],
but AcpJsonContext declares a [JsonSerializable] root per derived
transport. A declared derived root resolves its own contract without the
base converter, so serializing SseMcpServer (or any sibling) directly
wrote {"name",...,"url"} with no "type" discriminator and no version
gate, in both v1 and v2; a derived-root payload could not round-trip
through the v2 read path, which rejects a type-less object.
Repeat the converter on each derived record so every declared root
routes through the same polymorphic writer, and widen CanConvert to the
whole McpServer family because the generated context validates a root's
converter with an exact-type check.
Red-first evidence: the new declared-root test reproduced the bypass
(assert-throws saw no exception) before the converter attribute landed.
Fixes #226
Co-Authored-By: Claude Code <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The derived McpServer records now carry their own converter, so the source generator no longer walks their properties; the List<McpHttpHeader> and List<McpEnvVariable> infos that used to be emitted as nested types vanished from AcpJsonContext and ApiCompat flagged their public ListMcpHttpHeader / ListMcpEnvVariable members as CP0002 removals. Declare both roots explicitly so the shipped context keeps its surface. Co-Authored-By: Claude Code <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题
#226：SSE 写侧版本闸门挂在基类
McpServer的[JsonConverter]上，但AcpJsonContext为每个派生 transport 各声明了一个[JsonSerializable]根。声明根按派生类型解析契约、不经过基类转换器，于是直接以SseMcpServer（或任意兄弟类型）为根序列化时：type字段的{"name":…,"url":…}type判别器 → 该载荷无法 round-trip修法
最小改动、根因修复：四个派生 record 各自重复挂同一个
McpServerJsonConverter（声明根即走同一多态写侧）；转换器CanConvert放宽到整个McpServer家族（生成上下文对根转换器做精确类型校验）。不删声明的
[JsonSerializable]根——AcpJsonContext是已发布 SDK 的公开面，删除是 breaking。验证
McpServerV2_DeclaredDerivedRoots_RouteThroughTheVersionGatedConverter在修复前复现旁路（Assert.Throws等不到异常）SseV1OnlyMessage；v1 声明根写出type:"sse"；StdioMcpServer声明根 v2 写出type:"stdio"（可 round-trip）SalmonEgg.Acp.Tests全量 1322/1322 通过dotnet format --verify-no-changes干净Fixes #226
🤖 Generated with Claude Code