Skip to content

Refactor session timeouts and add V2 feature warnings#3974

Merged
yrobla merged 1 commit intomainfrom
issue-3866-v1
Mar 3, 2026
Merged

Refactor session timeouts and add V2 feature warnings#3974
yrobla merged 1 commit intomainfrom
issue-3866-v1

Conversation

@yrobla
Copy link
Copy Markdown
Contributor

@yrobla yrobla commented Mar 3, 2026

Extract hardcoded timeout values in session manager into package-level constants for better maintainability. Add startup warnings when SessionManagementV2 is configured with incompatible features.

Changes:

  • pkg/transport/session: Extract 5s and 30s timeouts to named constants (defaultOperationTimeout and cleanupOperationTimeout) to improve readability and make adjustments easier
  • pkg/vmcp/server: Warn operators at startup when SessionManagementV2 is enabled alongside unsupported features (optimizer mode, composite tools) to provide early feedback instead of silent feature loss

Addresses PR review feedback from @aponcedeleonch.

Closes: #3866

@yrobla yrobla requested a review from Copilot March 3, 2026 11:36
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 3, 2026
aponcedeleonch
aponcedeleonch previously approved these changes Mar 3, 2026
Copy link
Copy Markdown
Member

@aponcedeleonch aponcedeleonch left a comment

Choose a reason for hiding this comment

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

nice, thanks!

Copy link
Copy Markdown
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

Refactors session-storage operation timeouts into named constants and adds startup-time operator warnings when SessionManagementV2 is enabled alongside currently-unsupported features, to avoid silent feature loss.

Changes:

  • Extracted hardcoded 5s/30s context.WithTimeout durations in transportsession.Manager into package-level constants.
  • Added startup warnings when SessionManagementV2 is enabled with optimizer mode and/or composite tools configured.

Reviewed changes

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

File Description
pkg/vmcp/server/server.go Emits startup warnings for unsupported feature combinations when SessionManagementV2 is enabled.
pkg/transport/session/manager.go Replaces repeated hardcoded timeout literals with named constants for readability/maintainability.

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

Extract hardcoded timeout values in session manager into package-level
constants for better maintainability. Add startup warnings when
SessionManagementV2 is configured with incompatible features.

Changes:
- pkg/transport/session: Extract 5s and 30s timeouts to named constants
  (defaultOperationTimeout and cleanupOperationTimeout) to improve
  readability and make adjustments easier
- pkg/vmcp/server: Warn operators at startup when SessionManagementV2
  is enabled alongside unsupported features (optimizer mode, composite
  tools) to provide early feedback instead of silent feature loss

Addresses PR review feedback from @aponcedeleonch.

Closes: #3866
@github-actions github-actions bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Mar 3, 2026
@yrobla yrobla requested a review from Copilot March 3, 2026 11:45
Copy link
Copy Markdown
Member

@aponcedeleonch aponcedeleonch left a comment

Choose a reason for hiding this comment

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

nice catch!

Copy link
Copy Markdown
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 2 out of 2 changed files in this pull request and generated 3 comments.


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

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 53.57143% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.11%. Comparing base (a6ccdf1) to head (3b8c8b1).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/vmcp/server/server.go 42.85% 8 Missing and 4 partials ⚠️
pkg/transport/session/manager.go 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3974      +/-   ##
==========================================
- Coverage   68.13%   68.11%   -0.02%     
==========================================
  Files         437      437              
  Lines       44337    44363      +26     
==========================================
+ Hits        30208    30217       +9     
- Misses      11782    11791       +9     
- Partials     2347     2355       +8     

☔ 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.

@yrobla yrobla merged commit 6425554 into main Mar 3, 2026
62 of 63 checks passed
@yrobla yrobla deleted the issue-3866-v1 branch March 3, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[vMCP] Implement SessionManager and wire up behind feature flag (Phase 2)

4 participants