Skip to content

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Jan 20, 2026

vMCP was discarding _meta fields from backend MCP server responses, violating the MCP specification requirement to preserve protocol-level metadata. This prevented clients from receiving progress tokens, trace IDs, and custom backend metadata.

Large PR Justification

This is an atomic PR with complete functionality and testing. Cannot be split.

Fixes #2640

@github-actions github-actions bot added the size/L Large PR: 600-999 lines changed label Jan 20, 2026
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 20, 2026
@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 87.03704% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.12%. Comparing base (deb7fad) to head (d602e22).

Files with missing lines Patch % Lines
pkg/vmcp/client/client.go 80.95% 10 Missing and 2 partials ⚠️
pkg/vmcp/composer/workflow_engine.go 42.85% 3 Missing and 1 partial ⚠️
pkg/vmcp/server/adapter/handler_factory.go 93.33% 2 Missing and 1 partial ⚠️
pkg/vmcp/server/telemetry.go 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3354      +/-   ##
==========================================
+ Coverage   64.84%   65.12%   +0.27%     
==========================================
  Files         375      377       +2     
  Lines       36603    36715     +112     
==========================================
+ Hits        23737    23910     +173     
+ Misses      10995    10927      -68     
- Partials     1871     1878       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 20, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for preserving _meta fields from backend MCP server responses in the vMCP architecture. Previously, protocol-level metadata (progress tokens, trace IDs, custom backend metadata) was being discarded when forwarding responses to clients.

Changes:

  • Modified BackendClient interface to return wrapper types (ToolCallResult, ResourceReadResult, PromptGetResult) that include _meta fields
  • Updated client implementation to extract and preserve _meta from backend responses
  • Added conversion functions to map between vmcp and mcp Meta/Content types
  • Updated all mocks and tests to work with new return types

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
pkg/vmcp/types.go Added new wrapper types (Content, ToolCallResult, ResourceReadResult, PromptGetResult) with Meta fields
pkg/vmcp/server/telemetry.go Updated telemetry wrapper method signatures to use new return types
pkg/vmcp/server/adapter/handler_factory.go Added conversion functions and updated handlers to preserve _meta when forwarding to clients
pkg/vmcp/mocks/mock_backend_client.go Updated mock implementations for new method signatures
pkg/vmcp/client/client.go Modified CallTool, ReadResource, GetPrompt to extract and preserve _meta from backend responses
pkg/vmcp/client/meta_integration_test.go New integration test validating _meta preservation end-to-end
pkg/vmcp/composer/workflow_engine.go Updated to extract output from new ToolCallResult wrapper type
pkg/vmcp/server/integration_test.go Updated mocks to return new wrapper types
pkg/vmcp/server/adapter/handler_factory_test.go Updated all test expectations to use new wrapper types
pkg/vmcp/composer/*_test.go Updated multiple test files to use new ToolCallResult wrapper

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/L Large PR: 600-999 lines changed labels Jan 20, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 20, 2026
@github-actions github-actions bot dismissed their stale review January 20, 2026 16:02

Large PR justification has been provided. Thank you!

@github-actions
Copy link
Contributor

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

@yrobla yrobla force-pushed the issue-2640 branch 2 times, most recently from 2ad4a00 to 17e9b06 Compare January 21, 2026 11:50
vMCP was discarding _meta fields from backend MCP server responses,
violating the MCP specification requirement to preserve protocol-level
metadata. This prevented clients from receiving progress tokens, trace
IDs, and custom backend metadata.

Fixes #2640
@github-actions github-actions bot removed the size/XL Extra large PR: 1000+ lines changed label Jan 21, 2026
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Jan 21, 2026
@yrobla yrobla requested a review from Copilot January 21, 2026 11:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 21, 2026
@yrobla yrobla marked this pull request as draft January 21, 2026 12:50
Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

Code Review: Inline Comments

These are non-blocking observations from the code review. The overall implementation is solid - these are documentation suggestions and minor improvement notes for future consideration.

Summary:

  • 4 inline comments added
  • All non-blocking (INFO/LOW severity)
  • Focus on documentation and future improvements

Expect(err).ToNot(HaveOccurred(), "Tool call should succeed")
Expect(result).ToNot(BeNil())

By("Checking metadata preservation infrastructure")
Copy link
Contributor

Choose a reason for hiding this comment

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

[QUESTION] E2E Coverage: Should yardstick return _meta?

The test uses yardstick backend which may or may not return _meta. The test gracefully handles both cases, but cannot guarantee the _meta preservation code path is exercised in CI.

Would it be valuable to extend yardstick to always return _meta with known values (e.g., a trace ID)? This would provide deterministic E2E coverage for the metadata preservation feature.

@yrobla
Copy link
Contributor Author

yrobla commented Jan 21, 2026

thanks for your comments, i will address them. We also discussed the possibility of adding metadata to yardstick test server, so we can properly validate it

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Virtual MCP Server Not Preserving _meta Field in Responses

4 participants