Add Session and SessionFactory interfaces for vMCP (Phase 1)#3882
Add Session and SessionFactory interfaces for vMCP (Phase 1)#3882
Conversation
yrobla
requested review from
JAORMX,
amirejaz,
jerm-dro and
jhrozek
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 transformationAlternative:
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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3882 +/- ##
==========================================
+ Coverage 66.80% 66.97% +0.17%
==========================================
Files 444 447 +3
Lines 44503 44893 +390
==========================================
+ Hits 29730 30068 +338
- Misses 12445 12476 +31
- Partials 2328 2349 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds the Phase 1 vMCP session layer (pkg/vmcp/session) to support session-scoped backend client lifecycle per THV-0038 / issue #3865, including default implementations and unit tests.
Changes:
- Introduces a new
Sessioninterface embeddingtransportsession.Session, plus capability accessors and domain operations (tool/resource/prompt). - Adds a
SessionFactoryabstraction with a default factory that initializes backend clients in parallel with bounded concurrency and partial-failure behavior. - Implements a thread-safe
defaultSessionand a production HTTP-based backend connector, plus unit tests for the session/factory behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/vmcp/session/session.go | Defines the vMCP Session interface and documents the distributed/sticky-session constraint and dual-layer model. |
| pkg/vmcp/session/factory.go | Adds SessionFactory + default implementation with parallel backend initialization and routing table/capability aggregation. |
| pkg/vmcp/session/default_session.go | Implements the runtime session object with locking + in-flight tracking + Close semantics. |
| pkg/vmcp/session/connector.go | Implements the HTTP/SSE backend connector and capability discovery via MCP Initialize + list queries. |
| pkg/vmcp/session/default_session_test.go | Adds unit tests for interface composition, routing operations, Close semantics, and factory init behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
github-actions
bot
dismissed
their stale review
Large PR justification has been provided. Thank you!
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
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 transformationAlternative:
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.
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.
github-actions
bot
added
size/XL
Implements issue #3865 and RFC THV-0038 (session-scoped client lifecycle). This is a purely additive Phase 1 change introducing the session layer to pkg/vmcp/session/: - session.go: Session interface extending transportsession.Session, documenting the dual-layer model (serialisable metadata vs. non-serialisable runtime state) and the sticky-session constraint for distributed deployments. - factory.go: SessionFactory interface, BackendConnector injectable function type, and defaultSessionFactory. Backends are initialised in parallel using errgroup with bounded concurrency (default 10). Partial initialisation: failed backends are logged and skipped so the session proceeds with the remaining healthy backends. - default_session.go: Thread-safe defaultSession with a RWMutex + WaitGroup pattern that ensures wg.Add(1) is always called while holding the read lock, preventing races between in-flight operations and Close(). - connector.go: mcpConnectedBackend wrapping a persistent mark3labs mcp-go client, NewHTTPBackendConnector with a layered HTTP transport chain (auth → identity propagation → 100 MB response size cap), and initAndQueryCapabilities for the MCP Initialize handshake and tool/resource/prompt discovery. Closes: #3865
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 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.
jerm-dro
left a comment
jerm-dro
left a comment
There was a problem hiding this comment.
Approving because I know there are many more PRs to come and don't want to prevent progress, but please address the blockers.
Implements issue #3865 and RFC THV-0038 (session-scoped client lifecycle).
Phase 1 introduces pkg/vmcp/session/, which owns the lifetime of persistent
MCP connections to backend servers for the duration of a client session.
session.go: Session interface extending transportsession.Session, with the
dual-layer state model (serialisable metadata vs. non-serialisable runtime
state) and sticky-session constraint documented.
factory.go: SessionFactory and defaultSessionFactory. Backends are
initialised in parallel (errgroup, capped at 10 concurrent) with partial
failure tolerance — failed backends are skipped so the session proceeds
with the remaining healthy ones.
default_session.go: Thread-safe dispatch using RWMutex + WaitGroup. The
critical invariant is wg.Add(1) under RLock, which prevents a race between
in-flight operations and Close() draining the group.
connector.go: mcpConnectedBackend wrapping a persistent mcp-go client per
backend, with a layered HTTP transport chain (auth → identity propagation)
and an MCP Initialize handshake on connect.
Also extracted ConvertMCPContent, ConvertMCPContents, and
ConcatenateResourceContents into pkg/vmcp/conversion to eliminate
duplication between the client and session packages. Two bugs fixed in
pkg/vmcp/client in the process: io.LimitReader was incorrectly applied to
SSE (a single long-lived response body, not per-event), and tool input
schemas were being copied field-by-field rather than via a JSON round-trip,
silently dropping additionalProperties and future SDK fields.
Large PR Justification
This is a complete PR with isolated functionality an related tests. Cannot be split.
Closes: #3865