Cloud service cleanup callbacks / move to events (#6519)

* Cloud: use events in SettingsService

* Cloud: simplify AuthService events

* Cloud: convert CloudService to an EventEmitter

* Apply suggestions from code review

Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>

---------

Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>

Author: jr

packages/cloud/src/CloudService.ts

@@ -1,4 +1,5 @@
 import * as vscode from "vscode"
+import EventEmitter from "events"
 
 import type {
 	CloudUserInfo,
@@ -10,7 +11,7 @@ import type {
 } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 
-import { CloudServiceCallbacks } from "./types"
+import { CloudServiceEvents } from "./types"
 import type { AuthService } from "./auth"
 import { WebAuthService, StaticTokenAuthService } from "./auth"
 import type { SettingsService } from "./SettingsService"
@@ -19,25 +20,37 @@ import { StaticSettingsService } from "./StaticSettingsService"
 import { TelemetryClient } from "./TelemetryClient"
 import { ShareService, TaskNotFoundError } from "./ShareService"
 
-export class CloudService {
+type AuthStateChangedPayload = CloudServiceEvents["auth-state-changed"][0]
+type AuthUserInfoPayload = CloudServiceEvents["user-info"][0]
+type SettingsPayload = CloudServiceEvents["settings-updated"][0]
+
+export class CloudService extends EventEmitter<CloudServiceEvents> implements vscode.Disposable {
 	private static _instance: CloudService | null = null
 
 	private context: vscode.ExtensionContext
-	private callbacks: CloudServiceCallbacks
-	private authListener: () => void
+	private authStateListener: (data: AuthStateChangedPayload) => void
+	private authUserInfoListener: (data: AuthUserInfoPayload) => void
 	private authService: AuthService | null = null
+	private settingsListener: (data: SettingsPayload) => void
 	private settingsService: SettingsService | null = null
 	private telemetryClient: TelemetryClient | null = null
 	private shareService: ShareService | null = null
 	private isInitialized = false
 	private log: (...args: unknown[]) => void
 
-	private constructor(context: vscode.ExtensionContext, callbacks: CloudServiceCallbacks) {
+	private constructor(context: vscode.ExtensionContext, log?: (...args: unknown[]) => void) {
+		super()
+
 		this.context = context
-		this.callbacks = callbacks
-		this.log = callbacks.log || console.log
-		this.authListener = () => {
-			this.callbacks.stateChanged?.()
+		this.log = log || console.log
+		this.authStateListener = (data: AuthStateChangedPayload) => {
+			this.emit("auth-state-changed", data)
+		}
+		this.authUserInfoListener = (data: AuthUserInfoPayload) => {
+			this.emit("user-info", data)
+		}
+		this.settingsListener = (data: SettingsPayload) => {
+			this.emit("settings-updated", data)
 		}
 	}
 
@@ -57,26 +70,20 @@ export class CloudService {
 
 			await this.authService.initialize()
 
-			this.authService.on("attempting-session", this.authListener)
-			this.authService.on("inactive-session", this.authListener)
-			this.authService.on("active-session", this.authListener)
-			this.authService.on("logged-out", this.authListener)
-			this.authService.on("user-info", this.authListener)
+			this.authService.on("auth-state-changed", this.authStateListener)
+			this.authService.on("user-info", this.authUserInfoListener)
 
 			// Check for static settings environment variable.
 			const staticOrgSettings = process.env.ROO_CODE_CLOUD_ORG_SETTINGS
 
 			if (staticOrgSettings && staticOrgSettings.length > 0) {
 				this.settingsService = new StaticSettingsService(staticOrgSettings, this.log)
 			} else {
-				const cloudSettingsService = new CloudSettingsService(
-					this.context,
-					this.authService,
-					() => this.callbacks.stateChanged?.(),
-					this.log,
-				)
-
+				const cloudSettingsService = new CloudSettingsService(this.context, this.authService, this.log)
 				cloudSettingsService.initialize()
+
+				cloudSettingsService.on("settings-updated", this.settingsListener)
+
 				this.settingsService = cloudSettingsService
 			}
 
@@ -219,13 +226,13 @@ export class CloudService {
 
 	public dispose(): void {
 		if (this.authService) {
-			this.authService.off("attempting-session", this.authListener)
-			this.authService.off("inactive-session", this.authListener)
-			this.authService.off("active-session", this.authListener)
-			this.authService.off("logged-out", this.authListener)
-			this.authService.off("user-info", this.authListener)
+			this.authService.off("auth-state-changed", this.authStateListener)
+			this.authService.off("user-info", this.authUserInfoListener)
 		}
 		if (this.settingsService) {
+			if (this.settingsService instanceof CloudSettingsService) {
+				this.settingsService.off("settings-updated", this.settingsListener)
+			}
 			this.settingsService.dispose()
 		}
 
@@ -248,13 +255,13 @@ export class CloudService {
 
 	static async createInstance(
 		context: vscode.ExtensionContext,
-		callbacks: CloudServiceCallbacks = {},
+		log?: (...args: unknown[]) => void,
 	): Promise<CloudService> {
 		if (this._instance) {
 			throw new Error("CloudService instance already created")
 		}
 
-		this._instance = new CloudService(context, callbacks)
+		this._instance = new CloudService(context, log)
 		await this._instance.initialize()
 		return this._instance
 	}

packages/cloud/src/CloudSettingsService.ts

@@ -1,4 +1,5 @@
 import * as vscode from "vscode"
+import EventEmitter from "events"
 
 import {
 	ORGANIZATION_ALLOW_ALL,
@@ -8,32 +9,38 @@ import {
 } from "@roo-code/types"
 
 import { getRooCodeApiUrl } from "./Config"
-import type { AuthService } from "./auth"
+import type { AuthService, AuthState } from "./auth"
 import { RefreshTimer } from "./RefreshTimer"
 import type { SettingsService } from "./SettingsService"
 
 const ORGANIZATION_SETTINGS_CACHE_KEY = "organization-settings"
 
-export class CloudSettingsService implements SettingsService {
+export interface SettingsServiceEvents {
+	"settings-updated": [
+		data: {
+			settings: OrganizationSettings
+			previousSettings: OrganizationSettings | undefined
+		},
+	]
+}
+
+export class CloudSettingsService extends EventEmitter<SettingsServiceEvents> implements SettingsService {
 	private context: vscode.ExtensionContext
 	private authService: AuthService
 	private settings: OrganizationSettings | undefined = undefined
 	private timer: RefreshTimer
 	private log: (...args: unknown[]) => void
 
-	constructor(
-		context: vscode.ExtensionContext,
-		authService: AuthService,
-		callback: () => void,
-		log?: (...args: unknown[]) => void,
-	) {
+	constructor(context: vscode.ExtensionContext, authService: AuthService, log?: (...args: unknown[]) => void) {
+		super()
+
 		this.context = context
 		this.authService = authService
 		this.log = log || console.log
 
 		this.timer = new RefreshTimer({
 			callback: async () => {
-				return await this.fetchSettings(callback)
+				return await this.fetchSettings()
 			},
 			successInterval: 30000,
 			initialBackoffMs: 1000,
@@ -49,21 +56,24 @@ export class CloudSettingsService implements SettingsService {
 			this.removeSettings()
 		}
 
-		this.authService.on("active-session", () => {
-			this.timer.start()
-		})
+		this.authService.on("auth-state-changed", (data: { state: AuthState; previousState: AuthState }) => {
+			if (data.state === "active-session") {
+				this.timer.start()
+			} else if (data.previousState === "active-session") {
+				this.timer.stop()
 
-		this.authService.on("logged-out", () => {
-			this.timer.stop()
-			this.removeSettings()
+				if (data.state === "logged-out") {
+					this.removeSettings()
+				}
+			}
 		})
 
 		if (this.authService.hasActiveSession()) {
 			this.timer.start()
 		}
 	}
 
-	private async fetchSettings(callback: () => void): Promise<boolean> {
+	private async fetchSettings(): Promise<boolean> {
 		const token = this.authService.getSessionToken()
 
 		if (!token) {
@@ -97,9 +107,14 @@ export class CloudSettingsService implements SettingsService {
 			const newSettings = result.data
 
 			if (!this.settings || this.settings.version !== newSettings.version) {
+				const previousSettings = this.settings
 				this.settings = newSettings
 				await this.cacheSettings()
-				callback()
+
+				this.emit("settings-updated", {
+					settings: this.settings,
+					previousSettings,
+				})
 			}
 
 			return true
@@ -131,6 +146,7 @@ export class CloudSettingsService implements SettingsService {
 	}
 
 	public dispose(): void {
+		this.removeAllListeners()
 		this.timer.stop()
 	}
 }