Add SessionManager behind sessionManagementV2 flag (Phase 2)#3906
Add SessionManager behind sessionManagementV2 flag (Phase 2)#3906yrobla wants to merge 1 commit intoissue-3865-v1from
Conversation
yrobla
requested review from
ChrisJBurns,
JAORMX,
amirejaz,
blkt,
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.
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Implements RFC THV-0038 Phase 2 by introducing a feature-flagged vmcpSessionManager that bridges MultiSession/MultiSessionFactory to the MCP SDK SessionIdManager, enabling session-scoped backend client lifecycle while keeping the existing path unchanged when disabled.
Changes:
- Add
vmcpSessionManagerbehindSessionManagementV2to generate/validate/terminate sessions and register per-session tools. - Extend
MultiSessionFactorywithMakeSessionWithID()and addtransportsession.Manager.ReplaceSession()to support two-phase session creation. - Update discovery middleware to bypass Phase 1 routing-table reconstruction for Phase 2
MultiSessionsessions; add unit + integration tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/vmcp/server/session_manager.go | New Phase 2 session manager (Generate/CreateSession/Validate/Terminate + tool adaptation). |
| pkg/vmcp/server/server.go | Wires Phase 2 manager behind SessionManagementV2 and registers session tools in OnRegisterSession. |
| pkg/vmcp/session/factory.go | Adds MakeSessionWithID() and refactors shared makeSession() implementation. |
| pkg/transport/session/manager.go | Adds ReplaceSession() upsert for swapping placeholder → real session. |
| pkg/vmcp/discovery/middleware.go | Skips Phase 1 routing-table reconstruction when the stored session is a MultiSession. |
| pkg/vmcp/config/config.go | Adds sessionManagementV2 operational config flag. |
| pkg/vmcp/server/session_manager_test.go | Unit tests for Phase 2 session manager behavior. |
| pkg/transport/session/manager_test.go | Unit tests for ReplaceSession() behavior. |
| pkg/vmcp/server/session_management_v2_integration_test.go | End-to-end tests for v2 initialize + tool routing and “old path unaffected” case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
github-actions
bot
dismissed
their stale review
Large PR justification has been provided. Thank you!
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## issue-3865-v1 #3906 +/- ##
=================================================
+ Coverage 67.21% 67.29% +0.08%
=================================================
Files 449 450 +1
Lines 45220 45447 +227
=================================================
+ Hits 30393 30582 +189
- Misses 12459 12490 +31
- Partials 2368 2375 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 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
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 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
bot
added
size/XL
Implement RFC THV-0038 Phase 2: a vmcpSessionManager that bridges
the MultiSession/MultiSessionFactory domain logic to the MCP SDK's
SessionIdManager interface. The feature is gated behind a
sessionManagementV2 config flag (default false), leaving the
existing code path completely unchanged when disabled.
Key changes:
- pkg/vmcp/server/session_manager.go: new vmcpSessionManager with
two-phase creation — Generate() stores a placeholder, CreateSession()
(called from OnRegisterSession hook) replaces it with a fully-formed
MultiSession via MakeSessionWithID(). Terminate() calls Close() before
deleting to release backend connections. GetAdaptedTools() returns
SDK tools with session-scoped handlers delegating to CallTool().
- pkg/vmcp/session/factory.go: add MakeSessionWithID() to the
MultiSessionFactory interface; refactor MakeSession to share a private
makeSession() implementation.
- pkg/transport/session/manager.go: add ReplaceSession() upsert to
swap a placeholder with a fully-formed MultiSession.
- pkg/vmcp/config/config.go: add SessionManagementV2 bool field.
- pkg/vmcp/server/server.go: wire vmcpSessionManager when flag+factory
are set; add handleSessionRegistrationV2() hook handler.
- pkg/vmcp/discovery/middleware.go: skip Phase 1 routing-table
reconstruction for Phase 2 MultiSession sessions.
- Tests: unit tests for all new methods, ReplaceSession coverage in
transport/session, and an end-to-end integration test covering
initialize, tool call routing, and the old-path-unaffected case.
Closes: #3866
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 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.
2 participants
Implement RFC THV-0038 Phase 2: a vmcpSessionManager that bridges the MultiSession/MultiSessionFactory domain logic to the MCP SDK's SessionIdManager interface. The feature is gated behind a sessionManagementV2 config flag (default false), leaving the existing code path completely unchanged when disabled.
Key changes:
Closes: #3866
Large PR Justification
This is an atomic PR that covers an isolated functionality, and includes complete tests. Cannot be split.