Fix $ref pointer resolution after output schema wrapping#1435
Open
weinong wants to merge 1 commit intomodelcontextprotocol:mainfrom
Open
Fix $ref pointer resolution after output schema wrapping#1435weinong wants to merge 1 commit intomodelcontextprotocol:mainfrom
weinong wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
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 modelcontextprotocol#1434
weinong
force-pushed
the
fix/rewrite-ref-after-output-schema-wrapping
branch
from
2b469d0 to
9c68a58
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes structured output JSON Schema generation for tools that return non-object types by ensuring $ref JSON Pointers remain valid after the schema is wrapped under properties.result (addresses #1434).
Changes:
- Add
RewriteRefPointersto rewrite internal$refvalues (#/…and#) after non-object output schemas are wrapped underproperties.result. - Add tests that validate wrapped structured output schemas still validate and that all
$refpointers resolve.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/ModelContextProtocol.Core/Server/AIFunctionMcpServerTool.cs | Rewrites $ref JSON Pointer paths after wrapping non-object output schemas under properties.result. |
| tests/ModelContextProtocol.Tests/Server/McpServerToolTests.cs | Adds coverage for duplicated-type and recursive-type $ref scenarios to ensure rewritten pointers are resolvable post-wrapping. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+634
to
+657
| 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<string>() 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); |
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
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.
Summary
CreateOutputSchema()wraps a non-object schema underproperties.result, rewrite all internal$refJSON Pointers so they remain valid at their new location.StructuredOutput_WithDuplicateTypeRefs_RewritesRefPointersto verify the fix.Problem
System.Text.Json'sJsonSchemaExporterdeduplicates repeated types by emitting$refpointers with absolute JSON Pointer paths (e.g.,#/items/properties/foo/items). WhenCreateOutputSchema()wraps a non-object schema inside{ "type": "object", "properties": { "result": <original> } }, these$refpaths become unresolvable because the original schema has moved under#/properties/result.This breaks all tools that:
IEnumerable<T>,List<T>, arrays, primitives)UseStructuredContent = true$refdeduplication)When a client calls
tools/list, the broken schema causes JSON Schema validation errors. In MCP clients that don't isolate per-tool errors (e.g., the TypeScript SDK'scacheToolMetadata()), one broken tool schema prevents all tools from being enumerated.Fix
Add
RewriteRefPointers()— a recursive walk over the wrapped JSON tree that prepends/properties/resultto every$refvalue starting with#/. Called immediately after the wrapping block inCreateOutputSchema().Reproduction
A minimal repro is available as a gist: https://gist.github.com/weinong/7a750e9b99c846dadc4fa41fc6c856fc
Fixes #1434