Skip to content

Conversation

@jr
Copy link
Collaborator

@jr jr commented Aug 28, 2025

Important

Sync remoteControlEnabled setting with cloud, making cloud the source of truth, and improve error handling for cloud interactions.

  • Behavior:
    • remoteControlEnabled setting in ClineProvider.ts now fetched from cloud settings using CloudService.instance.getUserSettings().
    • In webviewMessageHandler.ts, updates to remoteControlEnabled now update cloud settings via CloudService.instance.updateUserSettings().
    • extension.ts updates ExtensionBridgeService with remoteControlEnabled from cloud settings.
  • Error Handling:
    • Improved error logging for cloud setting fetch failures in ClineProvider.ts and extension.ts.
  • Misc:
    • Removed default value for remoteControlEnabled in ClineProvider.ts.

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

@jr jr requested review from cte and mrubens as code owners August 28, 2025 22:37
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Aug 28, 2025
Copy link
Contributor

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

Thank you for your contribution! I've reviewed the changes and found several issues that need attention before this can be merged.


const remoteControlEnabled = isCloudAgent
? true
: (CloudService.instance.getUserSettings()?.settings?.extensionBridgeEnabled ?? false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional? The error handling here only logs the error but doesn't provide any fallback value for remoteControlEnabled. If getUserSettings() returns null/undefined, the remote control state could be out of sync.

Consider ensuring the fallback value is always applied.


const remoteControlEnabled = isCloudAgent
? true
: (CloudService.instance.getUserSettings()?.settings?.extensionBridgeEnabled ?? false)
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic for determining remoteControlEnabled is duplicated from lines 144-146. Could we extract this into a helper function to avoid duplication?

For example:

function getRemoteControlEnabled(isCloudAgent: boolean): boolean {
    return isCloudAgent ? true : (CloudService.instance.getUserSettings()?.settings?.extensionBridgeEnabled ?? false);
}

let remoteControlEnabled: boolean = false

try {
const cloudSettings = CloudService.instance.getUserSettings()
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing null check - what happens if CloudService is not initialized yet? The code uses optional chaining but doesn't verify that CloudService.instance exists.

Consider adding a hasInstance() check before accessing the instance.

extensionBridgeEnabled: message.bool ?? false,
})
} catch (error) {
provider.log(`Failed to update cloud settings for remote control: ${error}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

The error is only logged but doesn't provide user feedback. Should we show an error message to the user when cloud settings fail to update?

Consider adding a user-facing error notification.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 28, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Aug 29, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 29, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Sep 1, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 23, 2025
@github-project-automation github-project-automation bot moved this from PR [Changes Requested] to Done in Roo Code Roadmap Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request PR - Changes Requested size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants