-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Disable Roomote Control on logout #7976
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.
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
Conversation
There was a problem hiding this 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 that they correctly implement the feature to disable remote control on logout. The code is clear with good comments. I have a few suggestions for improvement, particularly around type safety.
| public static async connectOrDisconnect( | ||
| userInfo: CloudUserInfo | null, | ||
| remoteControlEnabled: boolean | undefined, | ||
| userInfo: CloudUserInfo, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The connectOrDisconnect() method now requires a non-null userInfo parameter, but it's being called from ClineProvider.remoteControlEnabled() where userInfo could be null. This type mismatch could lead to runtime errors.
Consider either:
- Making userInfo nullable here: userInfo: CloudUserInfo | null
- Or ensuring all callers check for null before calling this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no type mismatches; the build would fail otherwise.
After logging out we're not able to fetch the roomote control settings and we were short-circuiting the disconnect logic in the case that the setting fetch failed.
Important
Disable remote control on logout by updating
BridgeOrchestratorandClineProviderlogic and adding tests.BridgeOrchestrator.isEnabled()now returnsfalseifuserisnullorremoteControlEnabledisfalse.ClineProvider.remoteControlEnabled()disconnectsBridgeOrchestratorifenabledisfalseoruserInfoisnull.extension.tscallsprovider.remoteControlEnabled(false)on logout.extension.spec.tsto testauthStateChangedHandlercallsBridgeOrchestrator.disconnect()on logout.extension.tsfor clarity.This description was created by
for 9ef637c. You can customize this summary. It will automatically update as commits are pushed.