Fix resource relay to preserve text/blob distinction#4335
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4335 +/- ##
==========================================
- Coverage 69.08% 68.82% -0.26%
==========================================
Files 478 478
Lines 48432 48510 +78
==========================================
- Hits 33457 33388 -69
- Misses 12314 12380 +66
- Partials 2661 2742 +81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yrobla
left a comment
There was a problem hiding this comment.
Clean fix for real bugs - the old approach of decoding base64 blobs then re-stringifying was genuinely lossy and the multi-item flattening was losing per-item metadata. The overall direction is right and the test coverage is good.
Three things worth discussing before merging:
…adata The vMCP resource relay had two bugs: 1. Binary corruption: all resources were forced into TextResourceContents with string(rawBytes), corrupting non-UTF-8 blob data that had been base64-decoded then re-stringified. 2. Multi-content flattening: ConcatenateResourceContents merged all items into a single []byte, losing per-item URIs and MIME types. Replace the lossy []byte + MimeType representation with []ResourceContent that preserves individual items with their text/blob distinction. Add bidirectional converters (ConvertMCPResourceContents / ToMCPResourceContents) and remove the old ConcatenateResourceContents function. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
22f0878 to
da9b5ef
Compare
- Document Text/Blob invariant on ResourceContent struct (Blob takes precedence when both are set) - Add discriminant comment in ToMCPResourceContents explaining the Blob-precedence logic - Restore warn log level for unknown resource content types (dropping data should be visible to operators, not silenced at debug level) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
da9b5ef to
ebd1fb2
Compare
jerm-dro
left a comment
There was a problem hiding this comment.
Correctness fix — the old code was lossy. Good that this also removes the DiscoveredCapabilitiesFromContext dependency from the resource handler (eliminates context-variable coupling, anti-pattern #1). Same conversion layer observation as #4336 applies here too.
Generated with Claude Code
Summary
The vMCP relay was concatenating all resource contents into a single
[]bytewith oneMimeType, losing the text/blob distinction, per-item URIs, and per-item MIME types when a resource returned multiple content items. This meant multi-content resources and blob resources were degraded through the relay.This changes
ResourceReadResultfromContents []byte+MimeType stringtoContents []ResourceContent, preserving each item's type (text vs blob), URI, MIME type, and data independently.ResourceContenttype with Text, Data, URI, MimeType fields tovmcp.typesConcatenateResourceContentswithConvertMCPResourceContents/ToMCPResourceContentsround-trip convertersType of change
Test plan
task test)task lint-fix)Changes
pkg/vmcp/types.goResourceContenttype, updateResourceReadResultpkg/vmcp/conversion/content.goConvertMCPResourceContents/ToMCPResourceContents, removeConcatenateResourceContentspkg/vmcp/conversion/conversion_test.gopkg/vmcp/client/client.gopkg/vmcp/server/adapter/handler_factory.gopkg/vmcp/server/adapter/handler_factory_test.gopkg/vmcp/server/sessionmanager/session_manager.gopkg/vmcp/session/internal/backend/mcp_session.goDoes this introduce a user-facing change?
Resource responses from virtual MCP servers now preserve individual content items with their text/blob types, URIs, and MIME types, instead of concatenating everything into a single byte slice.
Special notes for reviewers
This is one of three related PRs fixing vMCP relay fidelity:
Generated with Claude Code