Skip to content

Conversation

@cte
Copy link
Collaborator

@cte cte commented Oct 20, 2025

Important

Refactor bridge system to use a single ExtensionChannel, removing TaskChannel and simplifying event handling and task management.

  • Bridge System Refactor:
    • Removed TaskChannel and BaseChannel classes, consolidating functionality into ExtensionChannel.
    • Updated BridgeOrchestrator to manage connections and events through ExtensionChannel only.
    • Removed task-specific event handling from BridgeOrchestrator.
  • Event Handling:
    • Updated ExtensionChannel to handle all task-related events and commands, including Message, TaskModeSwitched, TaskAskResponded, and more.
    • Refactored event mapping and listener setup in ExtensionChannel.
  • Task Management:
    • Removed task subscription logic from Task class in Task.ts.
    • Updated ClineProvider to handle task lifecycle events directly.
  • Miscellaneous:
    • Removed TaskChannel tests and related imports.
    • Updated extension.ts to reflect changes in bridge connection management.

This description was created by Ellipsis for 86977d2. You can customize this summary. It will automatically update as commits are pushed.

@cte cte requested review from jr and mrubens as code owners October 20, 2025 22:37
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Oct 20, 2025
@roomote
Copy link

roomote bot commented Oct 20, 2025

Review Summary

Previously identified issues:

  • Remove commented-out code block in ExtensionChannel.ts (lines 177-205) - leftover from TaskChannel refactoring

New issues found:

  • Fix cleanup flow during manual disconnection - extensionChannel.cleanup() is not called when BridgeOrchestrator.disconnect() is invoked because SocketTransport removes listeners before disconnecting

Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Review complete. Found 1 minor issue to address before approval.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Oct 20, 2025
Comment on lines +170 to +173
onDisconnect: async () => {
await this.extensionChannel.onDisconnect()
await this.extensionChannel.cleanup(this.socketTransport.getSocket())
},
Copy link

Choose a reason for hiding this comment

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

The cleanup flow is broken during manual disconnection. When BridgeOrchestrator.disconnect() calls socketTransport.disconnect(), the SocketTransport removes all listeners (line 247-248 in SocketTransport.ts) before disconnecting, which prevents the onDisconnect callback from firing. This means extensionChannel.cleanup() is never called during manual disconnection, potentially causing resource leaks. The old code explicitly called cleanup() before disconnect() to avoid this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

2 participants