Skip to content

Switch from Redis to a Socket.IO bridge #6930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 11, 2025
Merged

Switch from Redis to a Socket.IO bridge #6930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export type TaskOptions = {
apiConfiguration: ProviderSettings
enableDiff?: boolean
enableCheckpoints?: boolean
enableTaskBridge?: boolean
fuzzyMatchThreshold?: number
consecutiveMistakeLimit?: number
task?: string
Expand All @@ -118,7 +119,6 @@ export type TaskOptions = {
parentTask?: Task
taskNumber?: number
onCreated?: (task: Task) => void
enableTaskBridge?: boolean
}
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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.


export class Task extends EventEmitter<TaskEvents> implements TaskLike {
Expand Down Expand Up @@ -239,7 +239,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
checkpointServiceInitializing = false

// Task Bridge
taskBridgeService?: TaskBridgeService
enableTaskBridge: boolean
taskBridgeService: TaskBridgeService | null = null
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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.


// Streaming
isWaitingForFirstChunk = false
Expand All @@ -264,6 +265,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
apiConfiguration,
enableDiff = false,
enableCheckpoints = true,
enableTaskBridge = false,
fuzzyMatchThreshold = 1.0,
consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
task,
Expand All @@ -274,7 +276,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
parentTask,
taskNumber = -1,
onCreated,
enableTaskBridge = false,
}: TaskOptions) {
super()

Expand Down Expand Up @@ -313,6 +314,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.globalStoragePath = provider.context.globalStorageUri.fsPath
this.diffViewProvider = new DiffViewProvider(this.cwd, this)
this.enableCheckpoints = enableCheckpoints
this.enableTaskBridge = enableTaskBridge

this.rootTask = rootTask
this.parentTask = parentTask
Expand Down Expand Up @@ -352,11 +354,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {

this.toolRepetitionDetector = new ToolRepetitionDetector(this.consecutiveMistakeLimit)

// Initialize TaskBridgeService only if enabled
if (enableTaskBridge) {
this.taskBridgeService = TaskBridgeService.getInstance()
}

onCreated?.(this)

if (startTask) {
Expand Down Expand Up @@ -982,9 +979,20 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// Start / Abort / Resume

private async startTask(task?: string, images?: string[]): Promise<void> {
if (this.taskBridgeService) {
await this.taskBridgeService.initialize()
await this.taskBridgeService.subscribeToTask(this)
if (this.enableTaskBridge && CloudService.hasInstance()) {
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

Code duplication: This TaskBridgeService initialization logic is duplicated in resumeTaskFromHistory(). Consider extracting it to a private helper method like initializeTaskBridge() to follow DRY principles.

if (!this.taskBridgeService) {
const bridgeConfig = await CloudService.instance.cloudAPI?.bridgeConfig()
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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.


if (bridgeConfig) {
this.taskBridgeService = await TaskBridgeService.createInstance({
...bridgeConfig,
})
}
}

if (this.taskBridgeService) {
await this.taskBridgeService.subscribeToTask(this)
}
}

// `conversationHistory` (for API) and `clineMessages` (for webview)
Expand Down Expand Up @@ -1038,9 +1046,20 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
}

private async resumeTaskFromHistory() {
if (this.taskBridgeService) {
await this.taskBridgeService.initialize()
await this.taskBridgeService.subscribeToTask(this)
if (this.enableTaskBridge && CloudService.hasInstance()) {
if (!this.taskBridgeService) {
const bridgeConfig = await CloudService.instance.cloudAPI?.bridgeConfig()

if (bridgeConfig) {
this.taskBridgeService = await TaskBridgeService.createInstance({
...bridgeConfig,
})
}
}

if (this.taskBridgeService) {
await this.taskBridgeService.subscribeToTask(this)
}
}

const modifiedClineMessages = await this.getSavedClineMessages()
Expand Down Expand Up @@ -1293,6 +1312,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.taskBridgeService
.unsubscribeFromTask(this.taskId)
.catch((error) => console.error("Error unsubscribing from task bridge:", error))
this.taskBridgeService = null
}

// Release any terminals associated with this task.
Expand Down
50 changes: 36 additions & 14 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,6 @@ export class ClineProvider
throw new OrganizationAllowListViolationError(t("common:errors.violated_organization_allowlist"))
}

// Determine if TaskBridge should be enabled
const enableTaskBridge = isRemoteControlEnabled(cloudUserInfo, remoteControlEnabled)

const task = new Task({
provider: this,
apiConfiguration,
Expand All @@ -741,7 +738,7 @@ export class ClineProvider
parentTask,
taskNumber: this.clineStack.length + 1,
onCreated: this.taskCreationCallback,
enableTaskBridge,
enableTaskBridge: isRemoteControlEnabled(cloudUserInfo, remoteControlEnabled),
...options,
})

Expand Down Expand Up @@ -2139,26 +2136,49 @@ export class ClineProvider
* Handle remote control enabled/disabled state changes
* Manages ExtensionBridgeService and TaskBridgeService lifecycle
*/
public async handleRemoteControlToggle(enabled: boolean): Promise<void> {
public async handleRemoteControlToggle(enabled: boolean) {
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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.

const {
CloudService: CloudServiceImport,
ExtensionBridgeService,
TaskBridgeService,
} = await import("@roo-code/cloud")

const userInfo = CloudServiceImport.instance.getUserInfo()

// Handle ExtensionBridgeService using static method
await ExtensionBridgeService.handleRemoteControlState(userInfo, enabled, this, (message: string) =>
this.log(message),
const bridgeConfig = await CloudServiceImport.instance.cloudAPI?.bridgeConfig()

if (!bridgeConfig) {
this.log("[CloudService] Failed to get bridge config")
return
}

ExtensionBridgeService.handleRemoteControlState(
userInfo,
enabled,
{ ...bridgeConfig, provider: this },
(message: string) => this.log(message),
)

if (isRemoteControlEnabled(userInfo, enabled)) {
// Set up TaskBridgeService for the currently active task if one exists
// Set up TaskBridgeService for the currently active task if one exists.
const currentTask = this.getCurrentCline()
if (currentTask && !currentTask.taskBridgeService) {

if (currentTask && !currentTask.taskBridgeService && CloudService.hasInstance()) {
try {
currentTask.taskBridgeService = TaskBridgeService.getInstance()
await currentTask.taskBridgeService.subscribeToTask(currentTask)
if (!currentTask.taskBridgeService) {
const bridgeConfig = await CloudService.instance.cloudAPI?.bridgeConfig()

if (bridgeConfig) {
currentTask.taskBridgeService = await TaskBridgeService.createInstance({
...bridgeConfig,
})
}
}

if (currentTask.taskBridgeService) {
await currentTask.taskBridgeService.subscribeToTask(currentTask)
}

this.log(`[TaskBridgeService] Subscribed current task ${currentTask.taskId} to TaskBridge`)
} catch (error) {
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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.

const message = `[TaskBridgeService#subscribeToTask] ${error instanceof Error ? error.message : String(error)}`
Expand All @@ -2167,12 +2187,12 @@ export class ClineProvider
}
}
} else {
// Disconnect TaskBridgeService for all tasks in the stack
// Disconnect TaskBridgeService for all tasks in the stack.
for (const task of this.clineStack) {
if (task.taskBridgeService) {
try {
await task.taskBridgeService.unsubscribeFromTask(task.taskId)
task.taskBridgeService = undefined
task.taskBridgeService = null
this.log(`[TaskBridgeService] Unsubscribed task ${task.taskId} from TaskBridge`)
} catch (error) {
const message = `[TaskBridgeService#unsubscribeFromTask] for task ${task.taskId}: ${error instanceof Error ? error.message : String(error)}`
Expand All @@ -2181,6 +2201,8 @@ export class ClineProvider
}
}
}

TaskBridgeService.resetInstance()
}
}

Expand Down
18 changes: 12 additions & 6 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,21 @@ export async function activate(context: vscode.ExtensionContext) {
cloudService.on("auth-state-changed", postStateListener)
cloudService.on("settings-updated", postStateListener)

cloudService.on("user-info", ({ userInfo }) => {
cloudService.on("user-info", async ({ userInfo }) => {
postStateListener()

// Check if remote control is enabled in user settings
const remoteControlEnabled = contextProxy.getValue("remoteControlEnabled")
const bridgeConfig = await cloudService.cloudAPI?.bridgeConfig()

// Handle ExtensionBridgeService state using static method
ExtensionBridgeService.handleRemoteControlState(userInfo, remoteControlEnabled, provider, (message: string) =>
outputChannel.appendLine(message),
if (!bridgeConfig) {
outputChannel.appendLine("[CloudService] Failed to get bridge config")
return
}

ExtensionBridgeService.handleRemoteControlState(
userInfo,
contextProxy.getValue("remoteControlEnabled"),
{ ...bridgeConfig, provider },
Copy link
Contributor

@roomote roomote bot Aug 11, 2025

Choose a reason for hiding this comment

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

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?

(message: string) => outputChannel.appendLine(message),
)
})

Expand Down
Loading