Remove session v1 and SessionManagementV2 feature flag#4128
Open
Remove session v1 and SessionManagementV2 feature flag#4128
Conversation
yrobla
requested review from
ChrisJBurns,
amirejaz,
jerm-dro and
jhrozek
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the legacy “session v1” implementation and the SessionManagementV2 feature flag so that session-scoped backend lifecycle (MultiSession) is the only supported session flow across the server, CLI, and operator.
Changes:
- Delete v1 session types/adapters and the
sessionManagementV2config/CRD field; makeserver.Config.SessionFactoryrequired. - Simplify naming by dropping “V2” terminology across code/tests and always enabling session-scoped routing behavior.
- Update operator + E2E/integration/unit tests to match the single session-management path (including unconditional HMAC secret handling).
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Renames/updates E2E coverage to reflect the single session-management path and removal of the flag. |
| pkg/vmcp/session/vmcp_session_test.go | Deletes v1 VMCPSession unit tests. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation. |
| pkg/vmcp/session/factory.go | Removes migration-era TODO wording; aligns comments with “standard” aggregation path. |
| pkg/vmcp/session/admission.go | Adds package-level doc comment. |
| pkg/vmcp/server/testfactory_test.go | Adds minimal SessionFactory helper for server-package tests. |
| pkg/vmcp/server/status_test.go | Updates server construction to provide required SessionFactory. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Renames tests/helpers to remove v1/v2 naming. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Updates docs to remove flag wording (session manager is now always used). |
| pkg/vmcp/server/session_manager_interface.go | Removes feature-flag wording from interface docs. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames helpers/tests away from “V2”; updates config usage. |
| pkg/vmcp/server/session_management_integration_test.go | Renames fake session/factory types and server builders away from “V2”; removes old-path test. |
| pkg/vmcp/server/session_adapter_test.go | Deletes v1 session ID adapter tests. |
| pkg/vmcp/server/session_adapter.go | Deletes v1 session ID adapter implementation. |
| pkg/vmcp/server/server_test.go | Updates tests to pass SessionFactory; adjusts Accept-header validation test flow. |
| pkg/vmcp/server/server.go | Makes SessionFactory required, always uses sessionmanager + session-scoped routing, removes v1 capability injection path, adds BackendClient accessor for tests. |
| pkg/vmcp/server/readiness_test.go | Updates server construction to provide required SessionFactory. |
| pkg/vmcp/server/integration_test.go | Refactors integration tests to supply MultiSession factories and support telemetry/audit assertions under session management. |
| pkg/vmcp/server/health_test.go | Updates server construction to provide required SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates server construction to provide required SessionFactory (via minimal test factory). |
| pkg/vmcp/discovery/middleware_test.go | Adjusts tests to use streamable session factory + MultiSession test doubles for routing-table injection. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback logic; assumes MultiSession for subsequent requests; updates docs for composite routing context. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes DeepCopy for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults_test.go | Removes tests asserting SessionManagementV2 default/opt-out behavior. |
| pkg/vmcp/config/defaults.go | Removes SessionManagementV2 defaults/mergo pointer-preservation logic. |
| pkg/vmcp/config/config.go | Removes OperationalConfig.SessionManagementV2 field from API type. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates docs to remove v1/v2 wording. |
| pkg/vmcp/aggregator/aggregator.go | Updates interface docs to remove v2 wording. |
| docs/operator/crd-api.md | Removes sessionManagementV2 from generated CRD API docs. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 schema from Helm CRD template. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 schema from packaged CRD. |
| cmd/vmcp/app/commands.go | Always constructs SessionFactory; removes flag gating and improves option assembly. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always mounts HMAC secret env var (no conditional flag). |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures HMAC secret exists (removes opt-out guard). |
Comments suppressed due to low confidence (2)
pkg/vmcp/server/session_management_realbackend_integration_test.go:39
- Comment typo: "with session management and and a" has a duplicated "and".
test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go:307 - This ginkgo.By message still references the removed SessionManagementV2 flag. Since the rest of the test has been renamed to generic "session management", consider updating this string to avoid confusing E2E logs.
💡 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
github-actions
bot
added
size/XL
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the legacy v1 session implementation and the SessionManagementV2 feature flag, making session-scoped backend lifecycle the only supported path throughout vMCP (server, operator, config/CRDs, and tests).
Changes:
- Deletes v1 session types/adapter paths and standardizes naming by dropping “V2” terminology across code/tests/fixtures.
- Makes
server.Config.SessionFactorymandatory and routes all session handling through the session-scoped manager. - Removes the
sessionManagementV2CRD/config field and updates operator reconciliation + e2e/integration/unit tests accordingly.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Renames/updates e2e coverage to the single session-management path; removes opt-out scenario. |
| pkg/vmcp/session/vmcp_session_test.go | Deletes v1 VMCPSession unit tests. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation and helpers. |
| pkg/vmcp/session/token_binding_test.go | Updates tests to call MakeSessionWithID and provide explicit IDs. |
| pkg/vmcp/session/factory.go | Removes MakeSession convenience method; standardizes routing-table aggregation commentary. |
| pkg/vmcp/session/default_session_test.go | Updates tests to use MakeSessionWithID + generated IDs. |
| pkg/vmcp/session/default_session.go | Updates lifecycle docs to reference MakeSessionWithID. |
| pkg/vmcp/session/connector_integration_test.go | Updates integration tests to use MakeSessionWithID + generated IDs. |
| pkg/vmcp/session/admission.go | Adds package-level doc comment for session utilities. |
| pkg/vmcp/server/testfactory_test.go | Adds minimal SessionFactory helper for server-internal tests. |
| pkg/vmcp/server/status_test.go | Ensures tests provide required SessionFactory in server.Config. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Removes MakeSession from fakes; renames tests to non-V2 naming. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Updates package docs to remove feature-flag references. |
| pkg/vmcp/server/session_manager_interface.go | Updates interface docs to remove feature-flag references. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames helpers/tests to non-V2 naming; removes flag usage in config. |
| pkg/vmcp/server/session_management_integration_test.go | Renames fake types/helpers and removes “old path unused” test. |
| pkg/vmcp/server/session_adapter_test.go | Deletes v1 session ID adapter tests. |
| pkg/vmcp/server/session_adapter.go | Deletes v1 session ID adapter implementation. |
| pkg/vmcp/server/server_test.go | Updates tests to always supply SessionFactory; adjusts Accept-header test to avoid blocking. |
| pkg/vmcp/server/server.go | Makes SessionFactory required; removes feature flag branches; always uses session-scoped routing + session manager. |
| pkg/vmcp/server/readiness_test.go | Updates readiness tests to provide SessionFactory. |
| pkg/vmcp/server/integration_test.go | Introduces test factories/sessions to support audit/telemetry integration tests under session-scoped lifecycle. |
| pkg/vmcp/server/health_test.go | Updates health tests to provide SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates monitoring tests to provide a minimal SessionFactory. |
| pkg/vmcp/discovery/middleware_test.go | Updates middleware tests to store routing info on MultiSession rather than v1 sessions. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback logic; injects routing context from MultiSession; updates initialize behavior for session-scoped routing. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes deepcopy handling for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults_test.go | Removes default/opt-out tests for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults.go | Removes defaulting logic and mergo workarounds for deleted SessionManagementV2 field. |
| pkg/vmcp/config/config.go | Deletes the OperationalConfig.SessionManagementV2 field. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates docs to remove “v1 vs v2” wording. |
| pkg/vmcp/aggregator/aggregator.go | Updates docs to remove “v2” wording. |
| docs/operator/crd-api.md | Removes sessionManagementV2 field from generated CRD API docs. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from CRD schema template. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from CRD schema artifact. |
| cmd/vmcp/app/commands.go | Always creates a SessionFactory; removes flag gate; makes aggregator option conditional. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always injects the HMAC secret env var for token binding. |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures HMAC secret creation (removes opt-out guard). |
Comments suppressed due to low confidence (1)
pkg/vmcp/server/session_management_realbackend_integration_test.go:39
- Comment typo: "with session management and and a" has a duplicated "and".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
Session v2 is now the sole implementation. The SessionManagementV2
config option has been deleted along with all v1 code paths:
VMCPSession, sessionIDAdapter, the global-router-based discovery
fallback, and related tests. SessionFactory is now a required field
on server.Config. All "V2" naming has been dropped — factories,
managers, tests, and e2e fixtures now use plain "session" terminology.
Dead v1 capability-injection helpers (injectCapabilities,
injectOptimizerCapabilities, setSessionResourcesDirect) have been
removed, as has the operator's conditional HMAC-secret guard.
Affected: pkg/vmcp/server, pkg/vmcp/session, pkg/vmcp/config,
pkg/vmcp/discovery, cmd/vmcp, cmd/thv-operator, test/e2e
Related-to: #3872
github-actions
bot
added
size/XL
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy “session v1” implementation and the SessionManagementV2 feature flag, making the session-scoped backend lifecycle the only supported session management mode across the vMCP server, CLI, operator, and test suites.
Changes:
- Deletes all v1 session code paths and related tests (including
VMCPSession/ adapter-based session management and v1 opt-out behavior). - Makes
SessionFactorymandatory invmcpserver.Configand wires session-scoped routing as the default. - Renames “V2” terminology throughout code/tests/fixtures to plain “session” naming, and updates operator/CRD/docs accordingly.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/vmcp/helpers/vmcp_server.go | Ensures integration test server config includes a real SessionFactory (with aggregator consistency). |
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Updates e2e suite to remove flag usage and align naming with single session implementation. |
| pkg/vmcp/session/vmcp_session_test.go | Deletes v1 session tests. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation and helpers. |
| pkg/vmcp/session/token_binding_test.go | Updates tests to use MakeSessionWithID and explicit IDs. |
| pkg/vmcp/session/factory.go | Removes MakeSession (auto-ID) from factory interface; standardizes on MakeSessionWithID. |
| pkg/vmcp/session/default_session_test.go | Updates factory tests to generate IDs explicitly and call MakeSessionWithID. |
| pkg/vmcp/session/default_session.go | Updates lifecycle docs to reflect MakeSessionWithID creation path. |
| pkg/vmcp/session/connector_integration_test.go | Updates integration tests to use MakeSessionWithID. |
| pkg/vmcp/session/admission.go | Adds package-level documentation for session utilities. |
| pkg/vmcp/server/testfactory_test.go | Adds minimal SessionFactory helper for server package tests. |
| pkg/vmcp/server/status_test.go | Updates server test config to include required SessionFactory. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Removes MakeSession from fake factory; renames tests away from “VMCPSessionManager”. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Updates package docs to remove feature-flag framing. |
| pkg/vmcp/server/session_manager_interface.go | Updates docs to remove feature-flag framing. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames “V2” helper/tests and removes flag usage. |
| pkg/vmcp/server/session_management_integration_test.go | Renames “V2” fakes/helpers/tests; removes old-path coverage. |
| pkg/vmcp/server/session_adapter_test.go | Deletes v1 session ID adapter tests. |
| pkg/vmcp/server/session_adapter.go | Deletes v1 session ID adapter implementation. |
| pkg/vmcp/server/server_test.go | Updates server tests to always provide SessionFactory; adjusts Accept-header test to avoid blocking. |
| pkg/vmcp/server/server.go | Makes SessionFactory required, always enables session-scoped routing, removes v1 capability injection path, adds test accessor. |
| pkg/vmcp/server/readiness_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/server/integration_test.go | Updates infra-focused integration tests to use session factories and routing tables; adds telemetry-aware session factory wiring. |
| pkg/vmcp/server/health_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates health monitoring tests to include required SessionFactory. |
| pkg/vmcp/discovery/middleware_test.go | Updates discovery middleware tests to use StreamableSession placeholders and MultiSession routing-table injection. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback and enforces MultiSession invariant for subsequent requests; always enables session-scoped routing. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes deepcopy logic for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults_test.go | Removes defaults and opt-out tests for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults.go | Removes defaulting/mergo special-casing for deleted SessionManagementV2. |
| pkg/vmcp/config/config.go | Removes SessionManagementV2 from OperationalConfig schema. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates comments to remove “v1/v2” wording. |
| pkg/vmcp/aggregator/aggregator.go | Updates interface docs to remove “v2” wording. |
| docs/operator/crd-api.md | Removes sessionManagementV2 from published CRD API docs. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 field from CRD template. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 field from packaged CRD. |
| cmd/vmcp/app/commands.go | Always creates SessionFactory; removes flag-driven factory creation logic. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always mounts HMAC secret env var for token binding. |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures HMAC secret exists (removes opt-out guard). |
Comments suppressed due to low confidence (1)
pkg/vmcp/server/session_management_realbackend_integration_test.go:40
- The comment has a duplicated word: "session management and and a". Please fix the typo to avoid confusion in test documentation.
💡 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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4128 +/- ##
==========================================
+ Coverage 68.97% 69.03% +0.05%
==========================================
Files 461 459 -2
Lines 46555 46338 -217
==========================================
- Hits 32112 31990 -122
+ Misses 11971 11892 -79
+ Partials 2472 2456 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the legacy “session v1” implementation and the SessionManagementV2 feature flag so that session-scoped backend lifecycle (MultiSession) is the only supported path across server, operator, and tests.
Changes:
- Deletes v1 session types/adapter code paths and makes
SessionFactoryrequired onvmcpserver.Config. - Renames “V2” terminology to plain “session” throughout unit/integration/e2e tests and helpers.
- Updates operator CRDs/docs and controller logic to remove the flag and always manage/mount the HMAC secret.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/vmcp/helpers/vmcp_server.go | Builds a real SessionFactory (with aggregator) for integration-test server construction. |
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Removes feature-flag usage and v1 opt-out e2e coverage; renames suite text/fixtures. |
| pkg/vmcp/session/vmcp_session_test.go | Deletes v1 VMCPSession unit tests. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation/helpers. |
| pkg/vmcp/session/types/session.go | Adds go:generate to produce a MultiSession mock. |
| pkg/vmcp/session/types/mocks/mock_session.go | Adds generated GoMock for MultiSession. |
| pkg/vmcp/session/token_binding_test.go | Updates tests to use MakeSessionWithID (explicit IDs) instead of removed MakeSession. |
| pkg/vmcp/session/factory.go | Removes MakeSession API and updates comments to reflect the single session path. |
| pkg/vmcp/session/default_session_test.go | Migrates tests to MakeSessionWithID and explicit UUID generation. |
| pkg/vmcp/session/default_session.go | Updates lifecycle docs to reference MakeSessionWithID. |
| pkg/vmcp/session/connector_integration_test.go | Updates integration tests to use MakeSessionWithID and explicit UUIDs. |
| pkg/vmcp/session/admission.go | Adds/clarifies package-level docs for session utilities. |
| pkg/vmcp/server/testfactory_test.go | Adds minimal SessionFactory helper for server package tests. |
| pkg/vmcp/server/status_test.go | Supplies required SessionFactory in server construction for status tests. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Removes MakeSession from fake factory and renames tests to non-V2 terminology. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Drops “V2” wording in package docs; session manager remains two-phase. |
| pkg/vmcp/server/session_manager_interface.go | Updates interface docs to remove feature-flag wording. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames helpers/tests to non-V2 terminology and updates config fields. |
| pkg/vmcp/server/session_management_integration_test.go | Reworks fake session factory to align with required SessionFactory and mock-based sessions. |
| pkg/vmcp/server/session_adapter_test.go | Deletes v1 sessionIDAdapter tests. |
| pkg/vmcp/server/session_adapter.go | Deletes v1 sessionIDAdapter implementation. |
| pkg/vmcp/server/server_test.go | Updates tests to always provide SessionFactory; adjusts Accept-header test to avoid streaming hangs. |
| pkg/vmcp/server/server.go | Removes feature flag, requires SessionFactory, always uses sessionmanager + session-scoped discovery routing. |
| pkg/vmcp/server/readiness_test.go | Updates tests to provide SessionFactory. |
| pkg/vmcp/server/integration_test.go | Refactors integration tests to use MultiSession-based factories and routing tables. |
| pkg/vmcp/server/health_test.go | Updates tests to provide SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates tests to provide minimal SessionFactory. |
| pkg/vmcp/server/export_test.go | Exposes BackendClient to external test packages (test-only build). |
| pkg/vmcp/discovery/middleware_test.go | Updates session manager factory and uses MultiSession mock for routing-table injection. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback logic; injects routing context only from MultiSession routing table. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes deep-copy support for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults_test.go | Removes defaults tests for deleted SessionManagementV2. |
| pkg/vmcp/config/defaults.go | Removes defaulting and merge-workaround logic for deleted SessionManagementV2. |
| pkg/vmcp/config/config.go | Removes sessionManagementV2 from OperationalConfig API. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates comments to remove v1/v2 wording. |
| pkg/vmcp/aggregator/aggregator.go | Updates docs to remove v1/v2 wording in interface comments. |
| docs/operator/crd-api.md | Removes sessionManagementV2 from generated CRD API docs. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from Helm CRD template schema. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from packaged CRD schema. |
| cmd/vmcp/app/commands.go | Always creates SessionFactory; removes flag-conditioned wiring and unused aggregator helper. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always mounts HMAC secret env var (no flag gating). |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures HMAC secret exists (no flag gating). |
Comments suppressed due to low confidence (1)
pkg/vmcp/server/session_management_realbackend_integration_test.go:39
- Comment has a duplicated word: "session management and and a". Please remove the extra "and" for clarity.
💡 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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the vMCP session-management migration by removing the legacy “session v1” implementation and the SessionManagementV2 feature flag, making session-scoped lifecycle the only supported path end-to-end (server, CLI/config, operator/CRDs, and tests).
Changes:
- Deletes v1 session code paths and the
SessionManagementV2config/CRD surface area;server.Config.SessionFactoryis now required. - Standardizes naming by dropping “V2” terminology across session/server code and tests, and updates operator behavior to always provision/mount the HMAC secret.
- Updates integration/e2e/unit tests to use the session factory path, adding generated GoMock mocks where needed.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/integration/vmcp/helpers/vmcp_server.go | Ensures integration test servers pass a SessionFactory configured with the server’s aggregator. |
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Removes feature-flag-driven e2e cases and renames suite to reflect single session-management path. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation. |
| pkg/vmcp/session/vmcp_session_test.go | Removes tests for deleted v1 session type. |
| pkg/vmcp/session/types/session.go | Adds go:generate for MultiSession mock generation. |
| pkg/vmcp/session/types/mocks/mock_session.go | Adds generated GoMock for MultiSession. |
| pkg/vmcp/session/mocks/mock_factory.go | Adds generated GoMock for MultiSessionFactory. |
| pkg/vmcp/session/factory.go | Removes MakeSession from factory interface and updates comments/behavior to center MakeSessionWithID. |
| pkg/vmcp/session/default_session.go | Updates lifecycle docs to match MakeSessionWithID. |
| pkg/vmcp/session/default_session_test.go | Updates tests to supply explicit IDs via MakeSessionWithID. |
| pkg/vmcp/session/token_binding_test.go | Updates token-binding tests to call MakeSessionWithID and generate UUIDs in-test. |
| pkg/vmcp/session/connector_integration_test.go | Updates integration tests to use MakeSessionWithID. |
| pkg/vmcp/session/admission.go | Expands package comment to reflect broader session-management scope. |
| pkg/vmcp/server/server.go | Removes feature flag logic; always uses sessionmanager-backed session ID manager and requires SessionFactory. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Updates package docs to remove feature-flag references (core behavior unchanged). |
| pkg/vmcp/server/session_manager_interface.go | Updates interface docs to remove feature-flag references. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Updates tests/names to match single session manager path and removes MakeSession usage. |
| pkg/vmcp/server/session_adapter.go | Deletes legacy SDK session-id adapter (v1 path). |
| pkg/vmcp/server/session_adapter_test.go | Removes tests for deleted adapter. |
| pkg/vmcp/server/session_management_integration_test.go | Reworks session-management integration tests to use GoMock-based session factory/session instead of bespoke fakes. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames helpers/tests away from “V2” terminology and removes feature-flag config. |
| pkg/vmcp/server/testfactory_test.go | Adds minimal SessionFactory helper for server package tests. |
| pkg/vmcp/server/telemetry_integration_test.go | Moves/rewrites telemetry integration test to ensure outgoing backend metrics are exercised under session-scoped routing. |
| pkg/vmcp/server/server_test.go | Updates tests to always provide a SessionFactory; improves Accept-header test to avoid blocking on streaming handlers. |
| pkg/vmcp/server/status_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/server/readiness_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/server/integration_test.go | Refactors integration test fakes to work with the required SessionFactory and session-scoped routing. |
| pkg/vmcp/server/health_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates server construction to include required SessionFactory. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback and injects routing context from MultiSession for composite tool routing. |
| pkg/vmcp/discovery/middleware_test.go | Updates middleware tests to store a MultiSession (mock) in the transport session manager. |
| pkg/vmcp/config/config.go | Removes SessionManagementV2 from operational config schema. |
| pkg/vmcp/config/defaults.go | Removes defaulting logic for deleted SessionManagementV2 flag. |
| pkg/vmcp/config/defaults_test.go | Removes tests for deleted defaulting behavior. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes deepcopy handling for deleted field. |
| pkg/vmcp/aggregator/aggregator.go | Updates docs to remove “v2” wording. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates docs to remove “v2” wording. |
| docs/operator/crd-api.md | Removes sessionManagementV2 from documented CRD API. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from Helm CRD template. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 from rendered CRD file. |
| cmd/vmcp/app/commands.go | Always constructs a session factory; simplifies aggregator option handling; removes feature-flag wiring. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always mounts the HMAC secret env var (no conditional flag). |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures the HMAC secret exists (no opt-out). |
Comments suppressed due to low confidence (1)
pkg/vmcp/server/session_management_realbackend_integration_test.go:39
- There is a duplicated word in the comment: “session management and and a”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the vMCP session-management cleanup by removing the legacy “session v1” implementation and the SessionManagementV2 feature flag/config surface, making the session-scoped backend lifecycle the only supported path.
Changes:
- Removes v1 session types/paths (e.g.,
VMCPSession, session adapter) and deletes theSessionManagementV2operational config + CRD/docs references. - Makes
SessionFactorymandatory onvmcp/server.Config, and standardizes naming by dropping “V2” terminology across code/tests/fixtures. - Expands/updates session-scoped plumbing and tests (e.g., adds session-scoped resource adaptation in
sessionmanager, refreshes unit/integration/e2e coverage and mocks).
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/integration/vmcp/helpers/vmcp_server.go | Updates integration helper to construct and pass a required SessionFactory (with aggregator) into server config. |
| test/e2e/thv-operator/virtualmcp/virtualmcp_session_management_test.go | Renames/removes V2 flag usage in operator e2e tests and deletes explicit opt-out context. |
| pkg/vmcp/session/vmcp_session_test.go | Deletes v1 VMCPSession tests. |
| pkg/vmcp/session/vmcp_session.go | Deletes v1 VMCPSession implementation and helpers. |
| pkg/vmcp/session/types/session.go | Adds go:generate for MultiSession mocks. |
| pkg/vmcp/session/types/mocks/mock_session.go | Adds generated MultiSession GoMock implementation. |
| pkg/vmcp/session/token_binding_test.go | Updates token-binding tests to use MakeSessionWithID (explicit IDs). |
| pkg/vmcp/session/mocks/mock_factory.go | Adds generated MultiSessionFactory GoMock implementation. |
| pkg/vmcp/session/factory.go | Removes MakeSession from MultiSessionFactory; adds go:generate and updates comments/naming to “standard” path. |
| pkg/vmcp/session/default_session_test.go | Updates session factory tests to use MakeSessionWithID and explicit UUIDs. |
| pkg/vmcp/session/default_session.go | Updates lifecycle docs to reference MakeSessionWithID. |
| pkg/vmcp/session/connector_integration_test.go | Updates integration tests to use MakeSessionWithID with explicit UUIDs. |
| pkg/vmcp/session/admission.go | Clarifies package comment for session utilities. |
| pkg/vmcp/server/testfactory_test.go | Adds a minimal SessionFactory for server-internal tests that only require non-nil config. |
| pkg/vmcp/server/telemetry_integration_test.go | Adds/relocates telemetry integration coverage for session-scoped telemetry behavior. |
| pkg/vmcp/server/status_test.go | Updates server construction to provide required SessionFactory. |
| pkg/vmcp/server/sessionmanager/session_manager_test.go | Refactors session-manager tests to use generated mocks; adds coverage for adapted resources. |
| pkg/vmcp/server/sessionmanager/session_manager.go | Adds GetAdaptedResources for session-scoped resource handlers. |
| pkg/vmcp/server/session_manager_interface.go | Extends the server SessionManager interface to include GetAdaptedResources. |
| pkg/vmcp/server/session_management_realbackend_integration_test.go | Renames helpers/tests to drop V2 terminology and removes the flag usage. |
| pkg/vmcp/server/session_management_integration_test.go | Updates integration tests to mock session factories/sessions; removes old-path assertions tied to the deleted flag. |
| pkg/vmcp/server/session_adapter_test.go | Deletes tests for the removed v1 sessionIDAdapter. |
| pkg/vmcp/server/session_adapter.go | Deletes the v1 sessionIDAdapter implementation. |
| pkg/vmcp/server/server_test.go | Updates server tests to pass required SessionFactory and adjusts Accept-header test execution to avoid streaming hangs. |
| pkg/vmcp/server/server.go | Removes flag gating; makes SessionFactory required; switches placeholder sessions to StreamableSession; always enables session-scoped routing middleware; injects session-scoped resources. |
| pkg/vmcp/server/readiness_test.go | Updates readiness tests to pass required SessionFactory. |
| pkg/vmcp/server/integration_test.go | Removes v1 routing-table reproducer and updates audit/other integration tests to use session-scoped factories/mocks. |
| pkg/vmcp/server/health_test.go | Updates server construction to pass required SessionFactory. |
| pkg/vmcp/server/health_monitoring_test.go | Updates health-monitoring tests to pass required SessionFactory (minimal factory). |
| pkg/vmcp/discovery/middleware_test.go | Updates discovery middleware tests to use StreamableSession placeholders and MultiSession mocks for routing-table injection. |
| pkg/vmcp/discovery/middleware.go | Removes v1 fallback logic and focuses subsequent-request routing-context injection from MultiSession. |
| pkg/vmcp/config/zz_generated.deepcopy.go | Removes deepcopy handling for deleted SessionManagementV2 field. |
| pkg/vmcp/config/defaults_test.go | Removes tests tied to SessionManagementV2 defaults/opt-out behavior. |
| pkg/vmcp/config/defaults.go | Removes SessionManagementV2 defaulting/merge workaround logic. |
| pkg/vmcp/config/config.go | Removes SessionManagementV2 from the operational config API. |
| pkg/vmcp/aggregator/default_aggregator.go | Updates comments to remove v1/v2 wording. |
| pkg/vmcp/aggregator/aggregator.go | Updates comments to remove v1/v2 wording. |
| docs/operator/crd-api.md | Removes sessionManagementV2 from generated operator CRD API docs. |
| deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 field from Helm CRD template. |
| deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml | Removes sessionManagementV2 field from packaged CRD YAML. |
| cmd/vmcp/app/commands.go | Removes flag-based session-factory conditional; always creates session factory; avoids passing nil aggregator option. |
| cmd/thv-operator/controllers/virtualmcpserver_deployment.go | Always mounts HMAC-secret env var (no conditional based on removed flag). |
| cmd/thv-operator/controllers/virtualmcpserver_controller.go | Always ensures HMAC secret exists (removes opt-out guard tied to removed flag). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
size/XL
jerm-dro
approved these changes
Mar 13, 2026
Contributor
jerm-dro
left a comment
jerm-dro
left a comment
There was a problem hiding this comment.
🎉
This is safe to merge, since this release https://github.com/stacklok/toolhive/releases/tag/v0.12.0 include v2 by default.
Comment on lines
+284
to
+288
| // Backend tool calls are routed by session-scoped handlers registered with the SDK. | ||
| // However, composite tool workflow steps go through the shared router which requires | ||
| // DiscoveredCapabilities in the context. Inject capabilities built from the session's | ||
| // routing table so composite workflows can route backend tool calls correctly. | ||
| multiSess, isMulti := rawSess.(vmcpsession.MultiSession) |
Contributor
There was a problem hiding this comment.
let's leave a TODO: to remove composite tools dependency on the DiscoveredCapabilities context. Then, we can bring composite tools into the session abstraction to improve unit-testability and avoid the integration bugs you solved in your other PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Session v2 is now the sole implementation. The SessionManagementV2 config option has been deleted along with all v1 code paths: VMCPSession, sessionIDAdapter, the global-router-based discovery fallback, and related tests. SessionFactory is now a required field on server.Config. All "V2" naming has been dropped — factories, managers, tests, and e2e fixtures now use plain "session" terminology. Dead v1 capability-injection helpers (injectCapabilities, injectOptimizerCapabilities, setSessionResourcesDirect) have been removed, as has the operator's conditional HMAC-secret guard.
Fixes #3872
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Affected: pkg/vmcp/server, pkg/vmcp/session, pkg/vmcp/config,
pkg/vmcp/discovery, cmd/vmcp, cmd/thv-operator, test/e2e
Does this introduce a user-facing change?
Special notes for reviewers
Large PR Justification
This PR is a refactor, removing old and unused code, simplifying code paths and including complete test suite. Cannot be split.