Cloud: fix provider syncing #7603
Conversation
ClineProvider creation was moved before CloudService which broke the old way of doing things.
dosubot
bot
added
size:M
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for your contribution! I've reviewed the changes and this is a clean fix for the race condition between ClineProvider and CloudService initialization. The approach of deferring initialization until CloudService is ready is sound. I have some suggestions for improvement below.
|
|
||
| if (CloudService.hasInstance()) { | ||
| CloudService.instance.off("settings-updated", this.handleCloudSettingsUpdate) | ||
| CloudService.instance.on("settings-updated", this.handleCloudSettingsUpdate) |
There was a problem hiding this comment.
Is this intentional? The method removes and then immediately re-adds the "settings-updated" event listener (lines 321-322). This could lead to duplicate listeners if initializeCloudProfileSyncWhenReady() is called multiple times. Consider tracking whether a listener is already registered or using a flag to prevent duplicate registrations.
| /** | ||
| * Initialize cloud profile synchronization when CloudService is ready | ||
| * This method is called externally after CloudService has been initialized | ||
| */ |
There was a problem hiding this comment.
Consider enhancing this comment to explain why this deferred initialization pattern is necessary. For example: 'Initialize cloud profile synchronization when CloudService is ready. This method is called externally after CloudService has been initialized to handle the race condition where ClineProvider may be created before CloudService.'
| await provider.initializeCloudProfileSyncWhenReady() | ||
| } catch (error) { | ||
| outputChannel.appendLine( | ||
| `[CloudService] Failed to initialize cloud profile sync: ${error instanceof Error ? error.message : String(error)}`, |
There was a problem hiding this comment.
The error is caught and logged, but should the extension take any recovery action here? If cloud profile sync is critical for certain features, users might want to know if it fails during startup. Consider whether a user notification would be appropriate for this failure case.
hannesrudolph
added
the
Issue/PR - Triage
ClineProvidercreation was moved beforeCloudServicewhich broke the old way of doing things.Important
Fix cloud profile synchronization in
ClineProviderby deferring initialization untilCloudServiceis ready.ClineProvider, deferinitializeCloudProfileSync()untilCloudServiceis ready.initializeCloudProfileSyncWhenReady()inClineProviderto handle sync afterCloudServiceinitialization.extension.ts, callinitializeCloudProfileSyncWhenReady()afterCloudServiceis initialized.This description was created by
for b1f2608. You can customize this summary. It will automatically update as commits are pushed.