From 9c68a5822ac30eb7e4f998d5d60016895d05ecf1 Mon Sep 17 00:00:00 2001 From: Weinong Wang Date: Sun, 15 Mar 2026 17:57:23 +0000 Subject: [PATCH] Fix pointer resolution after output schema wrapping When CreateOutputSchema() wraps a non-object schema under properties.result, internal $ref JSON Pointers emitted by System.Text.Json's JsonSchemaExporter become unresolvable because they still point to the original (unwrapped) paths. Add RewriteRefPointers() to prepend /properties/result to every $ref that starts with #/, keeping the pointers valid after wrapping. Fixes #1434 --- .../Server/AIFunctionMcpServerTool.cs | 50 +++++ .../Server/McpServerToolTests.cs | 198 ++++++++++++++++++ 2 files changed, 248 insertions(+) diff --git a/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs b/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs index e91bdd206..c4307f1a5 100644 --- a/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs +++ b/src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs @@ -520,6 +520,11 @@ typeProperty.ValueKind is not JsonValueKind.String || ["required"] = new JsonArray { (JsonNode)"result" } }; + // After wrapping, any internal $ref pointers that used absolute JSON Pointer + // paths (e.g., "#/items/..." or "#") are now invalid because the original schema + // has moved under "#/properties/result". Rewrite them to account for the new location. + RewriteRefPointers(schemaNode["properties"]!["result"]); + structuredOutputRequiresWrapping = true; } @@ -529,6 +534,51 @@ typeProperty.ValueKind is not JsonValueKind.String || return outputSchema; } + /// + /// Recursively rewrites all $ref JSON Pointer values in the given node + /// to account for the schema having been wrapped under properties.result. + /// + /// + /// System.Text.Json's uses absolute + /// JSON Pointer paths (e.g., #/items/properties/foo) to deduplicate types that appear at + /// multiple locations in the schema. When the original schema is moved under + /// #/properties/result by the wrapping logic above, these pointers become unresolvable. + /// This method prepends /properties/result to every $ref that starts with #/, + /// and rewrites bare # (root self-references from recursive types) to #/properties/result, + /// so the pointers remain valid after wrapping. + /// + private static void RewriteRefPointers(JsonNode? node) + { + if (node is JsonObject obj) + { + if (obj.TryGetPropertyValue("$ref", out JsonNode? refNode) && + refNode?.GetValue() is string refValue) + { + if (refValue == "#") + { + obj["$ref"] = "#/properties/result"; + } + else if (refValue.StartsWith("#/", StringComparison.Ordinal)) + { + obj["$ref"] = "#/properties/result" + refValue.Substring(1); + } + } + + // ToList() creates a snapshot because the $ref assignment above may invalidate the enumerator. + foreach (var property in obj.ToList()) + { + RewriteRefPointers(property.Value); + } + } + else if (node is JsonArray arr) + { + foreach (var item in arr) + { + RewriteRefPointers(item); + } + } + } + private JsonElement? CreateStructuredResponse(object? aiFunctionResult) { if (ProtocolTool.OutputSchema is null) diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs index fd62a05c7..3ac13e949 100644 --- a/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs +++ b/tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs @@ -554,6 +554,180 @@ public async Task ToolWithNullableParameters_ReturnsExpectedSchema(JsonNumberHan Assert.True(JsonElement.DeepEquals(expectedSchema, tool.ProtocolTool.InputSchema)); } + [Fact] + public async Task StructuredOutput_WithDuplicateTypeRefs_RewritesRefPointers() + { + // When a non-object return type contains the same type at multiple locations, + // System.Text.Json's schema exporter emits $ref pointers for deduplication. + // After wrapping the schema under properties.result, those $ref pointers must + // be rewritten to remain valid. This test verifies that fix. + var data = new List + { + new() + { + WorkPhones = [new() { Number = "555-0100", Type = "work" }], + HomePhones = [new() { Number = "555-0200", Type = "home" }], + } + }; + + JsonSerializerOptions options = new() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; + McpServerTool tool = McpServerTool.Create(() => data, new() { Name = "tool", UseStructuredContent = true, SerializerOptions = options }); + var mockServer = new Mock(); + var request = new RequestContext(mockServer.Object, CreateTestJsonRpcRequest()) + { + Params = new CallToolRequestParams { Name = "tool" }, + }; + + var result = await tool.InvokeAsync(request, TestContext.Current.CancellationToken); + + Assert.NotNull(tool.ProtocolTool.OutputSchema); + Assert.Equal("object", tool.ProtocolTool.OutputSchema.Value.GetProperty("type").GetString()); + Assert.NotNull(result.StructuredContent); + + // Verify $ref pointers in the schema point to valid locations after wrapping. + // Without the fix, $ref values like "#/items/..." would be unresolvable because + // the original schema was moved under "#/properties/result". + AssertMatchesJsonSchema(tool.ProtocolTool.OutputSchema.Value, result.StructuredContent); + + // Also verify that any $ref in the schema starts with #/properties/result + // (confirming the rewrite happened). + string schemaJson = tool.ProtocolTool.OutputSchema.Value.GetRawText(); + var schemaNode = JsonNode.Parse(schemaJson)!; + AssertAllRefsStartWith(schemaNode, "#/properties/result"); + AssertAllRefsResolvable(schemaNode, schemaNode); + } + + [Fact] + public async Task StructuredOutput_WithRecursiveTypeRefs_RewritesRefPointers() + { + // When a non-object return type contains a recursive type, System.Text.Json's + // schema exporter emits $ref pointers (including potentially bare "#") for the + // recursive reference. After wrapping, these must be rewritten. For List, + // Children's items emit "$ref": "#/items" which must become "#/properties/result/items". + var data = new List + { + new() + { + Name = "root", + Children = [new() { Name = "child" }], + } + }; + + JsonSerializerOptions options = new() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; + McpServerTool tool = McpServerTool.Create(() => data, new() { Name = "tool", UseStructuredContent = true, SerializerOptions = options }); + var mockServer = new Mock(); + var request = new RequestContext(mockServer.Object, CreateTestJsonRpcRequest()) + { + Params = new CallToolRequestParams { Name = "tool" }, + }; + + var result = await tool.InvokeAsync(request, TestContext.Current.CancellationToken); + + Assert.NotNull(tool.ProtocolTool.OutputSchema); + Assert.Equal("object", tool.ProtocolTool.OutputSchema.Value.GetProperty("type").GetString()); + Assert.NotNull(result.StructuredContent); + + AssertMatchesJsonSchema(tool.ProtocolTool.OutputSchema.Value, result.StructuredContent); + + string schemaJson = tool.ProtocolTool.OutputSchema.Value.GetRawText(); + var schemaNode = JsonNode.Parse(schemaJson)!; + AssertAllRefsStartWith(schemaNode, "#/properties/result"); + AssertAllRefsResolvable(schemaNode, schemaNode); + } + + private static void AssertAllRefsStartWith(JsonNode? node, string expectedPrefix) + { + if (node is JsonObject obj) + { + if (obj.TryGetPropertyValue("$ref", out JsonNode? refNode) && + refNode?.GetValue() is string refValue) + { + Assert.StartsWith(expectedPrefix, refValue); + } + + foreach (var property in obj) + { + AssertAllRefsStartWith(property.Value, expectedPrefix); + } + } + else if (node is JsonArray arr) + { + foreach (var item in arr) + { + AssertAllRefsStartWith(item, expectedPrefix); + } + } + } + + /// + /// Walks the JSON tree and verifies that every $ref pointer resolves to a valid node. + /// + private static void AssertAllRefsResolvable(JsonNode root, JsonNode? node) + { + if (node is JsonObject obj) + { + if (obj.TryGetPropertyValue("$ref", out JsonNode? refNode) && + refNode?.GetValue() is string refValue && + refValue.StartsWith("#", StringComparison.Ordinal)) + { + var resolved = ResolveJsonPointer(root, refValue); + Assert.True(resolved is not null, $"$ref \"{refValue}\" does not resolve to a valid node in the schema."); + } + + foreach (var property in obj) + { + AssertAllRefsResolvable(root, property.Value); + } + } + else if (node is JsonArray arr) + { + foreach (var item in arr) + { + AssertAllRefsResolvable(root, item); + } + } + } + + /// + /// Resolves a JSON Pointer (e.g., #/properties/result/items) against a root node. + /// Returns null if the pointer cannot be resolved. + /// + private static JsonNode? ResolveJsonPointer(JsonNode root, string pointer) + { + if (pointer == "#") + { + return root; + } + + if (!pointer.StartsWith("#/", StringComparison.Ordinal)) + { + return null; + } + + JsonNode? current = root; + string[] segments = pointer.Substring(2).Split('/'); + foreach (string segment in segments) + { + if (current is JsonObject obj) + { + if (!obj.TryGetPropertyValue(segment, out current)) + { + return null; + } + } + else if (current is JsonArray arr && int.TryParse(segment, out int index) && index >= 0 && index < arr.Count) + { + current = arr[index]; + } + else + { + return null; + } + } + + return current; + } + public static IEnumerable StructuredOutput_ReturnsExpectedSchema_Inputs() { yield return new object[] { "string" }; @@ -679,6 +853,30 @@ Instance JSON document does not match the specified schema. record Person(string Name, int Age); + // Types used by StructuredOutput_WithDuplicateTypeRefs_RewritesRefPointers. + // ContactInfo has two properties of the same type (PhoneNumber) which causes + // System.Text.Json's schema exporter to emit $ref pointers for deduplication. + private sealed class PhoneNumber + { + public string? Number { get; set; } + public string? Type { get; set; } + } + + private sealed class ContactInfo + { + public List? WorkPhones { get; set; } + public List? HomePhones { get; set; } + } + + // Recursive type used by StructuredOutput_WithRecursiveTypeRefs_RewritesRefPointers. + // When List is the return type, Children's items emit "$ref": "#/items" + // pointing back to the first TreeNode definition, which must be rewritten after wrapping. + private sealed class TreeNode + { + public string? Name { get; set; } + public List? Children { get; set; } + } + [Fact] public void SupportsIconsInCreateOptions() {