Skip to content

Commit 2b469d0

Browse files
committed
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
1 parent aae77b1 commit 2b469d0

File tree

2 files changed

+123
-0
lines changed

2 files changed

+123
-0
lines changed

src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,11 @@ typeProperty.ValueKind is not JsonValueKind.String ||
520520
["required"] = new JsonArray { (JsonNode)"result" }
521521
};
522522

523+
// After wrapping, any internal $ref pointers that used absolute JSON Pointer
524+
// paths (e.g., "#/items/...") are now invalid because the original schema has
525+
// moved under "#/properties/result". Rewrite them to account for the new location.
526+
RewriteRefPointers(schemaNode);
527+
523528
structuredOutputRequiresWrapping = true;
524529
}
525530

@@ -529,6 +534,43 @@ typeProperty.ValueKind is not JsonValueKind.String ||
529534
return outputSchema;
530535
}
531536

537+
/// <summary>
538+
/// Recursively rewrites all <c>$ref</c> JSON Pointer values in the given node
539+
/// to account for the schema having been wrapped under <c>properties.result</c>.
540+
/// </summary>
541+
/// <remarks>
542+
/// <c>System.Text.Json</c>'s <see cref="System.Text.Json.Schema.JsonSchemaExporter"/> uses absolute
543+
/// JSON Pointer paths (e.g., <c>#/items/properties/foo</c>) to deduplicate types that appear at
544+
/// multiple locations in the schema. When the original schema is moved under
545+
/// <c>#/properties/result</c> by the wrapping logic above, these pointers become unresolvable.
546+
/// This method prepends <c>/properties/result</c> to every <c>$ref</c> that starts with <c>#/</c>
547+
/// so the pointers remain valid after wrapping.
548+
/// </remarks>
549+
private static void RewriteRefPointers(JsonNode? node)
550+
{
551+
if (node is JsonObject obj)
552+
{
553+
if (obj.TryGetPropertyValue("$ref", out JsonNode? refNode) &&
554+
refNode?.GetValue<string>() is string refValue &&
555+
refValue.StartsWith("#/", StringComparison.Ordinal))
556+
{
557+
obj["$ref"] = "#/properties/result" + refValue.Substring(1);
558+
}
559+
560+
foreach (var property in obj.ToList())
561+
{
562+
RewriteRefPointers(property.Value);
563+
}
564+
}
565+
else if (node is JsonArray arr)
566+
{
567+
foreach (var item in arr)
568+
{
569+
RewriteRefPointers(item);
570+
}
571+
}
572+
}
573+
532574
private JsonElement? CreateStructuredResponse(object? aiFunctionResult)
533575
{
534576
if (ProtocolTool.OutputSchema is null)

tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,72 @@ public async Task ToolWithNullableParameters_ReturnsExpectedSchema(JsonNumberHan
554554
Assert.True(JsonElement.DeepEquals(expectedSchema, tool.ProtocolTool.InputSchema));
555555
}
556556

557+
[Fact]
558+
public async Task StructuredOutput_WithDuplicateTypeRefs_RewritesRefPointers()
559+
{
560+
// When a non-object return type contains the same type at multiple locations,
561+
// System.Text.Json's schema exporter emits $ref pointers for deduplication.
562+
// After wrapping the schema under properties.result, those $ref pointers must
563+
// be rewritten to remain valid. This test verifies that fix.
564+
var data = new List<ContactInfo>
565+
{
566+
new()
567+
{
568+
WorkPhones = [new() { Number = "555-0100", Type = "work" }],
569+
HomePhones = [new() { Number = "555-0200", Type = "home" }],
570+
}
571+
};
572+
573+
JsonSerializerOptions options = new() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
574+
McpServerTool tool = McpServerTool.Create(() => data, new() { Name = "tool", UseStructuredContent = true, SerializerOptions = options });
575+
var mockServer = new Mock<McpServer>();
576+
var request = new RequestContext<CallToolRequestParams>(mockServer.Object, CreateTestJsonRpcRequest())
577+
{
578+
Params = new CallToolRequestParams { Name = "tool" },
579+
};
580+
581+
var result = await tool.InvokeAsync(request, TestContext.Current.CancellationToken);
582+
583+
Assert.NotNull(tool.ProtocolTool.OutputSchema);
584+
Assert.Equal("object", tool.ProtocolTool.OutputSchema.Value.GetProperty("type").GetString());
585+
Assert.NotNull(result.StructuredContent);
586+
587+
// Verify $ref pointers in the schema point to valid locations after wrapping.
588+
// Without the fix, $ref values like "#/items/..." would be unresolvable because
589+
// the original schema was moved under "#/properties/result".
590+
AssertMatchesJsonSchema(tool.ProtocolTool.OutputSchema.Value, result.StructuredContent);
591+
592+
// Also verify that any $ref in the schema starts with #/properties/result
593+
// (confirming the rewrite happened).
594+
string schemaJson = tool.ProtocolTool.OutputSchema.Value.GetRawText();
595+
var schemaNode = JsonNode.Parse(schemaJson)!;
596+
AssertAllRefsStartWith(schemaNode, "#/properties/result");
597+
}
598+
599+
private static void AssertAllRefsStartWith(JsonNode? node, string expectedPrefix)
600+
{
601+
if (node is JsonObject obj)
602+
{
603+
if (obj.TryGetPropertyValue("$ref", out JsonNode? refNode) &&
604+
refNode?.GetValue<string>() is string refValue)
605+
{
606+
Assert.StartsWith(expectedPrefix, refValue);
607+
}
608+
609+
foreach (var property in obj)
610+
{
611+
AssertAllRefsStartWith(property.Value, expectedPrefix);
612+
}
613+
}
614+
else if (node is JsonArray arr)
615+
{
616+
foreach (var item in arr)
617+
{
618+
AssertAllRefsStartWith(item, expectedPrefix);
619+
}
620+
}
621+
}
622+
557623
public static IEnumerable<object[]> StructuredOutput_ReturnsExpectedSchema_Inputs()
558624
{
559625
yield return new object[] { "string" };
@@ -679,6 +745,21 @@ Instance JSON document does not match the specified schema.
679745

680746
record Person(string Name, int Age);
681747

748+
// Types used by StructuredOutput_WithDuplicateTypeRefs_RewritesRefPointers.
749+
// ContactInfo has two properties of the same type (PhoneNumber) which causes
750+
// System.Text.Json's schema exporter to emit $ref pointers for deduplication.
751+
private sealed class PhoneNumber
752+
{
753+
public string? Number { get; set; }
754+
public string? Type { get; set; }
755+
}
756+
757+
private sealed class ContactInfo
758+
{
759+
public List<PhoneNumber>? WorkPhones { get; set; }
760+
public List<PhoneNumber>? HomePhones { get; set; }
761+
}
762+
682763
[Fact]
683764
public void SupportsIconsInCreateOptions()
684765
{

0 commit comments

Comments
 (0)