Switch from Redis to a Socket.IO bridge #6930
Conversation
dosubot
bot
added
the
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| taskNumber?: number | ||
| onCreated?: (task: Task) => void | ||
| enableTaskBridge?: boolean | ||
| } |
There was a problem hiding this comment.
Critical issue: enableTaskBridge is duplicated in the TaskOptions interface. It appears both on line 110 and line 122 (after being moved). This will cause TypeScript compilation errors. Please remove the duplicate on line 122.
| // Task Bridge | ||
| taskBridgeService?: TaskBridgeService | ||
| enableTaskBridge: boolean | ||
| taskBridgeService: TaskBridgeService | null = null |
There was a problem hiding this comment.
Inconsistent null/undefined handling. The code changes taskBridgeService from undefined to null in some places but not consistently. Consider standardizing on either null or undefined throughout for clarity. TypeScript's strict null checks work better with consistent usage.
| if (this.taskBridgeService) { | ||
| await this.taskBridgeService.initialize() | ||
| await this.taskBridgeService.subscribeToTask(this) | ||
| if (this.enableTaskBridge && CloudService.hasInstance()) { |
There was a problem hiding this comment.
Code duplication: This TaskBridgeService initialization logic is duplicated in resumeTaskFromHistory(). Consider extracting it to a private helper method like initializeTaskBridge() to follow DRY principles.
| await this.taskBridgeService.subscribeToTask(this) | ||
| if (this.enableTaskBridge && CloudService.hasInstance()) { | ||
| if (!this.taskBridgeService) { | ||
| const bridgeConfig = await CloudService.instance.cloudAPI?.bridgeConfig() |
There was a problem hiding this comment.
Missing error handling: If bridgeConfig() fails or returns null, there's no user-facing error handling. Consider adding a try-catch block and at least logging the error or notifying the user that remote control features may be unavailable.
| * Manages ExtensionBridgeService and TaskBridgeService lifecycle | ||
| */ | ||
| public async handleRemoteControlToggle(enabled: boolean): Promise<void> { | ||
| public async handleRemoteControlToggle(enabled: boolean) { |
There was a problem hiding this comment.
Return type annotation removed. The method signature changed from Promise<void> to no return type. For consistency and type safety, consider keeping the explicit return type.
| } | ||
|
|
||
| this.log(`[TaskBridgeService] Subscribed current task ${currentTask.taskId} to TaskBridge`) | ||
| } catch (error) { |
There was a problem hiding this comment.
Missing cleanup in error paths. If the TaskBridgeService creation succeeds but subscription fails, the service instance remains assigned but unusable. Consider setting it back to null in the catch block to ensure consistent state.
| ExtensionBridgeService.handleRemoteControlState( | ||
| userInfo, | ||
| contextProxy.getValue("remoteControlEnabled"), | ||
| { ...bridgeConfig, provider }, |
There was a problem hiding this comment.
Is this intentional? The provider variable is used here but it's defined later in the code (line 155). This might cause a reference error. Should this be inside a callback or should the provider be passed differently?
hannesrudolph
added
the
Issue/PR - Triage
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
Important
Switches from Redis to a Socket.IO bridge for task management, adding remote control features and UI updates.
TaskBridgeServiceinTask.tsto enable task subscription via Socket.IO bridge.enableTaskBridgeoption inTaskOptionsand integrates it in task lifecycle methods (startTask,resumeTaskFromHistory,abortTask).handleRemoteControlToggleinClineProvider.tsto manage remote control state.AccountView.tsxfor users withextensionBridgeEnabled.extension.tsto initializeExtensionBridgeServiceand handle remote control state changes.remoteControlEnabledtoExtensionStateinExtensionStateContext.tsx.AccountView.spec.tsxto verify remote control toggle visibility based on user settings.This description was created by
for 0bdc402. You can customize this summary. It will automatically update as commits are pushed.