fix(mcp-server): Handle oneOf documents as operation input/output roots - #1345
Open
psp65 wants to merge 1 commit into
Open
fix(mcp-server): Handle oneOf documents as operation input/output roots#1345psp65 wants to merge 1 commit into
psp65 wants to merge 1 commit into
Conversation
A document carrying the smithy.mcp#oneOf trait (a discriminated
polymorphic type) can appear as an operation's input or output in
bundled models, which are loaded with validation disabled. The
per-service schema cache is shared across schema kinds, so this
failed in one of two order-dependent ways:
- If a nested reference was processed first, the cache held a
JsonOneOfSchema and createJsonObjectSchema for the root then threw
ClassCastException, failing McpService construction entirely.
- If the root was processed first, the document rendered as an empty
object schema that was cached under the shape id, silently dropping
the oneOf variants from every nested reference.
Route oneOf documents in object positions through
createJsonOneOfSchema (preserving the cached oneOf schema for other
references) and re-shape the result into an object-typed schema:
JsonObjectSchema gains an optional oneOf member, producing
{"type": "object", "oneOf": [...]} as the MCP spec requires for tool
schemas. The guard is scoped to ShapeType.DOCUMENT (the trait's
selector), so any other shape kind carrying the trait keeps its
regular rendering, matching what runtime input/output adaptation
recognizes.
Operations are processed in sorted order, so the two regression tests
pin one processing order each; both fail on main (one with the
original ClassCastException) and pass with the fix.
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.
What behavior changes?
McpServiceno longer fails (or silently degrades) when a shape carrying thesmithy.mcp#oneOftrait is used directly as an operation input or output.With a model like:
building the
McpServicepreviously had two order-dependent failures, because the per-service schema cache is shared across schema kinds:ProcessShape) was processed first, the cache held aJsonOneOfSchemafor the shape, andcreateJsonObjectSchemafor theGetShapeoutput root then threwClassCastException: JsonOneOfSchema cannot be cast to JsonObjectSchema— failing construction of the entire service (every tool), not just the offending operation.createJsonObjectSchematreated the document as a plain (member-less) shape and cached an emptyJsonObjectSchemaunder the shape id, which nested references then reused — silently dropping all oneOf variants.After this change both positions render correctly: the root position produces
{"type": "object", "oneOf": [...]}(the MCP spec requires tool input/output schemas to be object-typed; the oneOf variants are all objects, so the constraints compose), and nested references keep the cachedJsonOneOfSchemaexactly as before.Why is this change needed?
Bundled models (loaded through
ModelBundles, which assembles with validation disabled) can and do contain document-typed operation outputs: tooling that converts service models for MCP represents polymorphic type hierarchies as@oneOfdocuments, and a service with a polymorphic operation output currently cannot list tools at all. Observed in production as a deterministicClassCastExceptionatMcpService.createJsonObjectSchemafor every such service.How was this validated?
McpServerTest, one per operation processing order (operations are processed in sorted order, so each order is pinned by operation naming):testOneOfDocumentAsOperationOutputRoot— root processed first: asserts the object-position rendering of the@oneOfdocument root AND that a nested reference to the same shape keeps its variants (the silent cache-pollution mode).testOneOfDocumentAsOperationOutputRootWithCachedSchema— nested reference processed first, so the root request hits a cachedJsonOneOfSchema(theClassCastExceptionmode).disableValidation()to mirrorModelBundles.mainwithout the fix and pass with it../gradlew :mcp:mcp-server:testpasses.What should reviewers focus on?
McpService#asJsonObjectSchema: the re-shaping of aJsonOneOfSchemainto an object-typed schema, and the new@oneOfguard increateJsonObjectSchemathat routes trait-carrying shapes throughcreateJsonOneOfSchemaso the cache keeps the full oneOf schema for other references.mcp-schemas/model/main.smithy:JsonObjectSchemagains an optionaloneOfmember so the object-typed root can carry the variants. The converted root preserves thetype: "object",oneOf, anddescriptionaJsonOneOfSchemawould serialize, plus the$schemaannotation that all object-typed roots already carry.createJsonObjectSchemais scoped toShapeType.DOCUMENT(the trait's selector), so a non-document shape incorrectly carrying the trait in an unvalidated model keeps its existing rendering — matching what runtime input/output adaptation recognizes.Additional Links
None.