Skip to content

Fix blob serialization in MCP output adaptation#1275

Open
DivyaMaddipudi wants to merge 1 commit into
smithy-lang:mainfrom
DivyaMaddipudi:fix-blob-serialization
Open

Fix blob serialization in MCP output adaptation#1275
DivyaMaddipudi wants to merge 1 commit into
smithy-lang:mainfrom
DivyaMaddipudi:fix-blob-serialization

Conversation

@DivyaMaddipudi

Copy link
Copy Markdown
Contributor

What behavior changes?

adaptOutputDocument no longer throws SerializationException: Expected a blob document, but found string when a downstream RPC v2 response contains a Blob field that arrives as an already-base64-encoded string Document. Instead, it passes the string through unchanged. Binary blob Documents continue to be base64-encoded as before.

Why is this change needed?

When MCP proxies a tools/call to a downstream Smithy service using RPC v2 CBOR, Blob fields in the response can be deserialized as string Documents (already base64-encoded) rather than typed blob Documents. The previous code unconditionally called doc.asBlob(), which throws when the document type is STRING.

How was this validated?

  • Unit tests added in McpServiceTest covering all three branches: STRING passthrough, BLOB encode, and unsupported type error
  • Structure-level test validating nested blob fields within a structure are adapted correctly
  • Full McpServerTest suite passes
  • End-to-end validation against a live MCP server

What should reviewers focus on?

  • McpService.java lines 1319–1324: the new nested switch on doc.type() inside the BLOB case
  • McpServiceTest.java: verify the test coverage is sufficient and the package-private visibility is acceptable

Additional Links


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

case BLOB -> Document.of(
Base64.getEncoder().encodeToString(ByteBufferUtils.getBytes(doc.asBlob())));
default -> badType(doc.type(), toType);
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a incorrect fix and in the wrong layer. The premise of this fix is also incorrect

to a downstream Smithy service using RPC v2 CBOR, Blob fields in the response can be deserialized as string Documents (already base64-encoded) rather than typed blob Documents.

That's the representation over the wire not after materialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants