diff --git a/changelog.d/unreleased/3055.fixed.md b/changelog.d/unreleased/3055.fixed.md new file mode 100644 index 0000000000..fd9d4c9b9a --- /dev/null +++ b/changelog.d/unreleased/3055.fixed.md @@ -0,0 +1,18 @@ +--- +category: fixed +issues: + - 3055 +affected: + - src/CodeIndex/Mcp/McpJsonNode.cs + - src/CodeIndex/Mcp/McpErrorEnvelope.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP JSON node cloning no longer serializes and reparses valid nodes (#3055)** — MCP response ids, session capabilities, progress tokens, and structured error extra data now use `JsonNode.DeepClone()` through a shared helper instead of `ToJsonString()` plus `JsonNode.Parse(...)`. + +## 日本語 + +- **MCP の JSON node clone が有効な node を文字列化して再 parse しなくなりました (#3055)** — MCP response id、session capabilities、progress token、structured error の追加データは、`ToJsonString()` と `JsonNode.Parse(...)` の組み合わせではなく、共有 helper 経由の `JsonNode.DeepClone()` を使うようになりました。 diff --git a/src/CodeIndex/Mcp/McpErrorEnvelope.cs b/src/CodeIndex/Mcp/McpErrorEnvelope.cs index c800a63f79..5366dcd657 100644 --- a/src/CodeIndex/Mcp/McpErrorEnvelope.cs +++ b/src/CodeIndex/Mcp/McpErrorEnvelope.cs @@ -76,7 +76,7 @@ public static JsonObject BuildData(string category, string suggestion, bool retr // canonical キーは extra でも上書きさせない。 if (kvp.Key is "category" or "suggestion" or "retry_safe") continue; - data[kvp.Key] = kvp.Value is null ? null : JsonNode.Parse(kvp.Value.ToJsonString()); + data[kvp.Key] = McpJsonNode.Clone(kvp.Value); } } return data; diff --git a/src/CodeIndex/Mcp/McpJsonNode.cs b/src/CodeIndex/Mcp/McpJsonNode.cs new file mode 100644 index 0000000000..42954af93a --- /dev/null +++ b/src/CodeIndex/Mcp/McpJsonNode.cs @@ -0,0 +1,9 @@ +using System.Text.Json.Nodes; + +namespace CodeIndex.Mcp; + +internal static class McpJsonNode +{ + public static JsonNode? Clone(JsonNode? node) + => node?.DeepClone(); +} diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 47007fab98..c88b152b4b 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -1088,7 +1088,7 @@ private bool TryCloneClientResponsePayload(JsonNode? payload, out JsonNode? clon if (!TryMeasureJsonUtf8BytesWithinLimit(payload, _jsonOptions, MaxClientResponseJsonBytes, out bytesWritten)) return false; - clone = payload.DeepClone(); + clone = McpJsonNode.Clone(payload); return true; } @@ -1212,7 +1212,7 @@ private static void ExtractResponseId(JsonNode request, out bool hasId, out Json if (request is JsonObject obj) { if (TryGetRequestId(obj, out hasId, out var requestId)) - id = requestId is null ? null : JsonNode.Parse(requestId.ToJsonString()); + id = McpJsonNode.Clone(requestId); else id = null; return; @@ -1916,23 +1916,51 @@ private void CaptureClientSession(JsonNode? initializeParams) private void CaptureClientCapabilities(JsonNode capabilities) { CaptureClientCapabilityFlags(capabilities); - var json = capabilities.ToJsonString(); - var serializedBytes = Encoding.UTF8.GetByteCount(json); - _clientCapabilitiesSerializedBytes = serializedBytes; - if (serializedBytes > MaxClientCapabilitiesJsonBytes) + if (!TryMeasureJsonUtf8BytesWithinLimit(capabilities, _jsonOptions, MaxClientCapabilitiesJsonBytes, out var serializedBytes)) { + _clientCapabilitiesSerializedBytes = serializedBytes; TruncateClientCapabilities("byte_limit"); return; } - try + _clientCapabilitiesSerializedBytes = serializedBytes; + if (!IsJsonNodeDepthWithinLimit(capabilities, MaxClientCapabilitiesDepth)) { - _clientCapabilities = JsonNode.Parse(json, documentOptions: new JsonDocumentOptions { MaxDepth = MaxClientCapabilitiesDepth }); + TruncateClientCapabilities("depth_limit"); + return; } - catch (JsonException) + + _clientCapabilities = McpJsonNode.Clone(capabilities); + } + + private static bool IsJsonNodeDepthWithinLimit(JsonNode node, int maxDepth) + => IsJsonNodeDepthWithinLimit(node, depth: 0, maxDepth); + + private static bool IsJsonNodeDepthWithinLimit(JsonNode? node, int depth, int maxDepth) + { + if (node is null) + return true; + if (depth > maxDepth) + return false; + + if (node is JsonObject obj) { - TruncateClientCapabilities("depth_limit"); + foreach (var kvp in obj) + { + if (!IsJsonNodeDepthWithinLimit(kvp.Value, depth + 1, maxDepth)) + return false; + } + } + else if (node is JsonArray array) + { + foreach (var item in array) + { + if (!IsJsonNodeDepthWithinLimit(item, depth + 1, maxDepth)) + return false; + } } + + return true; } private void TruncateClientCapabilities(string reason) @@ -1973,7 +2001,7 @@ private void ResetClientRoots() _clientRootsTruncated = false; } - internal JsonNode? ClientCapabilitiesForTests => _clientCapabilities is null ? null : JsonNode.Parse(_clientCapabilities.ToJsonString()); + internal JsonNode? ClientCapabilitiesForTests => McpJsonNode.Clone(_clientCapabilities); internal string[] ClientRootsForTests => _clientRoots .Select(root => root?.GetValue()) @@ -2426,7 +2454,7 @@ private static JsonObject CreateUnsupportedProtocolError(JsonNode? id, BoundedMc { ["jsonrpc"] = "2.0", ["error"] = error, - ["id"] = id is null ? JsonNode.Parse("null") : JsonNode.Parse(id.ToJsonString()) + ["id"] = McpJsonNode.Clone(id) }; return response; } @@ -2503,7 +2531,7 @@ internal static JsonObject CreateRateLimitedErrorResponse(JsonNode? id, string t { ["jsonrpc"] = "2.0", ["error"] = error, - ["id"] = id is null ? JsonNode.Parse("null") : JsonNode.Parse(id.ToJsonString()) + ["id"] = McpJsonNode.Clone(id) }; return response; } @@ -2790,7 +2818,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo return null; return TryMeasureJsonUtf8BytesWithinLimit(token, _jsonOptions, McpBoundedText.MaxProgressTokenJsonBytes, out _) - ? token.DeepClone() + ? McpJsonNode.Clone(token) : null; } @@ -2854,7 +2882,7 @@ private void EmitProgressNotification(JsonNode? progressToken, long progress, lo var parameters = new JsonObject { - ["progressToken"] = progressToken.DeepClone(), + ["progressToken"] = McpJsonNode.Clone(progressToken), ["progress"] = progress, }; if (total.HasValue) @@ -3578,7 +3606,7 @@ private static JsonObject CreateSuccessResponse(bool hasId, JsonNode? id, JsonNo ["result"] = result }; if (hasId) - response["id"] = id is null ? JsonNode.Parse("null") : JsonNode.Parse(id.ToJsonString()); + response["id"] = McpJsonNode.Clone(id); return response; } @@ -3676,7 +3704,7 @@ private static JsonObject CreateErrorResponse(bool hasId, JsonNode? id, int code } }; if (hasId) - response["id"] = id is null ? JsonNode.Parse("null") : JsonNode.Parse(id.ToJsonString()); + response["id"] = McpJsonNode.Clone(id); return response; } diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 91bc1c3758..5a7fcee9d1 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -868,6 +868,59 @@ public void Initialize_CapturesClientCapabilitiesAndRootsForSessionStatus() Assert.Equal("info", session["log_level"]!.GetValue()); } + [Fact] + public void Initialize_CapturesClientCapabilitiesAsDetachedClone_Issue3055() + { + var capabilities = new JsonObject + { + ["experimental"] = new JsonObject + { + ["progress"] = true, + }, + }; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "initialize", + ["params"] = new JsonObject + { + ["capabilities"] = capabilities, + }, + }; + + _server.HandleMessage(request); + capabilities["experimental"]!["progress"] = false; + var copy = _server.ClientCapabilitiesForTests!; + copy["experimental"]!["progress"] = false; + + Assert.True(_server.ClientCapabilitiesForTests!["experimental"]!["progress"]!.GetValue()); + } + + [Fact] + public void ErrorEnvelope_ClonesExtraDataWithoutSerializeParseRoundTrip_Issue3055() + { + var details = new JsonObject + { + ["ok"] = true, + }; + var extra = new JsonObject + { + ["details"] = details, + ["category"] = "shadow", + }; + + var data = McpErrorEnvelope.BuildData( + "custom_category", + "custom suggestion", + retrySafe: true, + extra); + details["ok"] = false; + + Assert.Equal("custom_category", data["category"]!.GetValue()); + Assert.True(data["details"]!["ok"]!.GetValue()); + } + [Fact] public void Initialize_CapsClientRootsForSessionStatus_Issue3076() {