Skip to content

Conversation

himanshusinghs
Copy link
Collaborator

Proposed changes

This is part of required changes needed for fixing multiple client connection handling issue on VSCode mongodb-js/vscode#1132.

The idea is for ConnectionManager to emit a close event when the session is being closed which is a signal that a previously connected client has disconnected.

The VSCode MCPController will make use of this event to clean up after client disconnects.

Checklist

@himanshusinghs himanshusinghs requested a review from a team as a code owner September 26, 2025 08:45
@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 08:45
@himanshusinghs himanshusinghs added the no-title-validation Add this label to disable the title check for this PR. label Sep 26, 2025
Copy link
Contributor

@Copilot 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

This PR adds a close event emission capability to the ConnectionManager to support proper cleanup when clients disconnect. The change is part of fixing multiple client connection handling issues in VSCode.

Key changes:

  • Adds a new close event to ConnectionManagerEvents interface
  • Implements a close() method in MCPConnectionManager that handles disconnection and emits the close event
  • Refactors Session's disconnect method to use the new close method instead of direct disconnect

Reviewed Changes

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

File Description
src/common/session.ts Simplifies disconnect method to use ConnectionManager's new close method
src/common/connectionManager.ts Adds close event, implements close method with proper error handling and event emission

@himanshusinghs himanshusinghs merged commit 73c4713 into main Sep 26, 2025
23 of 25 checks passed
@himanshusinghs himanshusinghs deleted the chore/emit-close-on-connection-manager branch September 26, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-title-validation Add this label to disable the title check for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants