Fix health-check close failing auth in vMCP BackendClient#4613
Fix health-check close failing auth in vMCP BackendClient#4613
Conversation
mcp-go's StreamableHTTP.Close() creates a DELETE request with context.Background(), discarding the health-check marker and identity from the original call context. The identityPropagatingRoundTripper only stored identity, so when ListCapabilities deferred c.Close(), the DELETE hit auth strategies with neither a health-check marker nor an identity, producing "no identity found in context" errors. Fix by capturing isHealthCheck at transport creation time and re-injecting both identity and the health-check marker into every outgoing request, including the synthetic DELETE from Close(). Fixes #4573
yrobla
requested review from
JAORMX,
amirejaz,
jerm-dro and
jhrozek
as code owners
There was a problem hiding this comment.
Pull request overview
This PR fixes vMCP backend session close requests failing authentication during health checks by ensuring the backend HTTP transport consistently re-injects the originating request’s identity (when present) and the health-check context marker onto all outgoing requests—including the synthetic DELETE request generated by mcp-go’s StreamableHTTP.Close().
Changes:
- Extend
identityPropagatingRoundTripperto propagate both identity and the health-check marker on every request. - Capture
isHealthCheckat client/transport creation time and re-apply it duringRoundTrip, coveringClose()’s context-less DELETE. - Add focused unit tests verifying identity propagation, health-check marker propagation, and propagation of both together.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/vmcp/client/client.go | Propagates health-check marker (in addition to identity) via transport to prevent auth failures on Close() DELETE. |
| pkg/vmcp/client/client_test.go | Adds unit tests validating identity + health-check marker propagation behavior in the transport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4613 +/- ##
==========================================
+ Coverage 68.85% 68.86% +0.01%
==========================================
Files 505 506 +1
Lines 52425 52434 +9
==========================================
+ Hits 36096 36109 +13
- Misses 13536 13537 +1
+ Partials 2793 2788 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
bot
added
size/S
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 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.
Summary
mcp-go's StreamableHTTP.Close() creates a DELETE request with context.Background(), discarding the health-check marker and identity from the original call context. The identityPropagatingRoundTripper only stored identity, so when ListCapabilities deferred c.Close(), the DELETE hit auth strategies with neither a health-check marker nor an identity, producing "no identity found in context" errors.
Fix by capturing isHealthCheck at transport creation time and re-injecting both identity and the health-check marker into every outgoing request, including the synthetic DELETE from Close().
Fixes #4573
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers