More cleanup

Author: cte

packages/cloud/src/AuthService.ts

@@ -6,6 +6,7 @@ import * as vscode from "vscode"
 
 import type { CloudUserInfo } from "@roo-code/types"
 
+import { CloudServiceCallbacks } from "./types"
 import { getClerkBaseUrl, getRooCodeApiUrl } from "./Config"
 import { RefreshTimer } from "./RefreshTimer"
 
@@ -22,27 +23,28 @@ type AuthState = "initializing" | "logged-out" | "active-session" | "inactive-se
 
 export class AuthService extends EventEmitter<AuthServiceEvents> {
 	private context: vscode.ExtensionContext
+	private userChanged: CloudServiceCallbacks["userChanged"]
+	private timer: RefreshTimer
 	private state: AuthState = "initializing"
 
 	private clientToken: string | null = null
 	private sessionToken: string | null = null
 	private sessionId: string | null = null
 
-	private timer: RefreshTimer
-
-	constructor(context: vscode.ExtensionContext, onUserInfo: (userInfo: CloudUserInfo | undefined) => void) {
+	constructor(context: vscode.ExtensionContext, userChanged: CloudServiceCallbacks["userChanged"]) {
 		super()
 
 		this.context = context
+		this.userChanged = userChanged
 
 		this.timer = new RefreshTimer({
 			callback: async () => {
-				await this.refreshSession(onUserInfo)
+				await this.refreshSession()
 				return true
 			},
-			successInterval: 50000,
-			initialBackoffMs: 1000,
-			maxBackoffMs: 300000,
+			successInterval: 50_000,
+			initialBackoffMs: 1_000,
+			maxBackoffMs: 300_000,
 		})
 	}
 
@@ -62,7 +64,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			this.clientToken = (await this.context.secrets.get(CLIENT_TOKEN_KEY)) || null
 			this.sessionId = this.context.globalState.get<string>(SESSION_ID_KEY) || null
 
-			// Determine initial state
+			// Determine initial state.
 			if (!this.clientToken || !this.sessionId) {
 				// TODO: it may be possible to get a new session with the client,
 				// but the obvious Clerk endpoints don't support that.
@@ -109,18 +111,14 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	 * @param code The authorization code from the callback
 	 * @param state The state parameter from the callback
 	 */
-	public async handleCallback(
-		code: string | null,
-		state: string | null,
-		onUserInfo?: (userInfo: CloudUserInfo | undefined) => void,
-	): Promise<void> {
+	public async handleCallback(code: string | null, state: string | null): Promise<void> {
 		if (!code || !state) {
 			vscode.window.showInformationMessage("Invalid Roo Code Cloud sign in url")
 			return
 		}
 
 		try {
-			// 1. Validate state parameter to prevent CSRF attacks
+			// Validate state parameter to prevent CSRF attacks.
 			const storedState = this.context.globalState.get(AUTH_STATE_KEY)
 
 			if (state !== storedState) {
@@ -142,11 +140,10 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 			this.emit("active-session", { previousState })
 			this.timer.start()
 
-			if (onUserInfo) {
-				this.getUserInfo().then(onUserInfo)
+			if (this.userChanged) {
+				this.getUserInfo().then(this.userChanged)
 			}
 
-			// Show success message
 			vscode.window.showInformationMessage("Successfully authenticated with Roo Code Cloud")
 			console.log("[auth] Successfully authenticated with Roo Code Cloud")
 		} catch (error) {
@@ -163,7 +160,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	 *
 	 * This method removes all stored tokens and stops the refresh timer.
 	 */
-	public async logout(onUserInfo?: (userInfo: CloudUserInfo | undefined) => void): Promise<void> {
+	public async logout(): Promise<void> {
 		try {
 			this.timer.stop()
 
@@ -185,8 +182,8 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 				await this.clerkLogout(oldClientToken, oldSessionId)
 			}
 
-			if (onUserInfo) {
-				this.getUserInfo().then(onUserInfo)
+			if (this.userChanged) {
+				this.getUserInfo().then(this.userChanged)
 			}
 
 			vscode.window.showInformationMessage("Logged out from Roo Code Cloud")
@@ -227,7 +224,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	 *
 	 * This method refreshes the session token using the client token.
 	 */
-	private async refreshSession(onUserInfo: (userInfo: CloudUserInfo | undefined) => void): Promise<void> {
+	private async refreshSession() {
 		if (!this.sessionId || !this.clientToken) {
 			console.log("[auth] Cannot refresh session: missing session ID or token")
 			this.state = "inactive-session"
@@ -240,7 +237,10 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 
 		if (previousState !== "active-session") {
 			this.emit("active-session", { previousState })
-			this.getUserInfo().then(onUserInfo)
+
+			if (this.userChanged) {
+				this.getUserInfo().then(this.userChanged)
+			}
 		}
 	}
 
@@ -383,15 +383,12 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 		return this._instance
 	}
 
-	static async createInstance(
-		context: vscode.ExtensionContext,
-		onUserInfo: (userInfo: CloudUserInfo | undefined) => void,
-	) {
+	static async createInstance(context: vscode.ExtensionContext, userChanged: CloudServiceCallbacks["userChanged"]) {
 		if (this._instance) {
 			throw new Error("AuthService instance already created")
 		}
 
-		this._instance = new AuthService(context, onUserInfo)
+		this._instance = new AuthService(context, userChanged)
 		await this._instance.initialize()
 		return this._instance
 	}

packages/cloud/src/CloudService.ts

@@ -1,21 +1,14 @@
 import * as vscode from "vscode"
-import EventEmitter from "events"
 
-import type { CloudUserInfo, TelemetryEvent } from "@roo-code/types"
+import type { CloudUserInfo, TelemetryEvent, OrganizationAllowList, OrganizationSettings } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 
-import { AuthService, type AuthServiceEvents } from "./AuthService"
+import { CloudServiceCallbacks } from "./types"
+import { AuthService } from "./AuthService"
 import { SettingsService } from "./SettingsService"
 import { TelemetryClient } from "./TelemetryClient"
 
-export type CloudServiceEvents = AuthServiceEvents
-
-export interface CloudServiceCallbacks {
-	onUserInfoChanged?: (userInfo: CloudUserInfo | undefined) => void
-	onSettingsChanged?: () => void
-}
-
-export class CloudService extends EventEmitter<CloudServiceEvents> {
+export class CloudService {
 	private static _instance: CloudService | null = null
 
 	private context: vscode.ExtensionContext
@@ -26,7 +19,6 @@ export class CloudService extends EventEmitter<CloudServiceEvents> {
 	private isInitialized = false
 
 	private constructor(context: vscode.ExtensionContext, callbacks: CloudServiceCallbacks) {
-		super()
 		this.context = context
 		this.callbacks = callbacks
 	}
@@ -38,14 +30,11 @@ export class CloudService extends EventEmitter<CloudServiceEvents> {
 
 		try {
 			this.authService = await AuthService.createInstance(this.context, (userInfo) => {
-				this.callbacks.onUserInfoChanged?.(userInfo)
+				this.callbacks.userChanged?.(userInfo)
 			})
 
-			this.authService.on("active-session", (args) => this.emit("active-session", args))
-			this.authService.on("logged-out", (args) => this.emit("logged-out", args))
-
 			this.settingsService = await SettingsService.createInstance(this.context, () =>
-				this.callbacks.onSettingsChanged?.(),
+				this.callbacks.settingsChanged?.(),
 			)
 
 			this.telemetryClient = new TelemetryClient(this.authService)
@@ -72,7 +61,7 @@ export class CloudService extends EventEmitter<CloudServiceEvents> {
 
 	public async logout(): Promise<void> {
 		this.ensureInitialized()
-		return this.authService!.logout((userInfo) => this.callbacks.onUserInfoChanged?.(userInfo))
+		return this.authService!.logout()
 	}
 
 	public isAuthenticated(): boolean {
@@ -102,17 +91,17 @@ export class CloudService extends EventEmitter<CloudServiceEvents> {
 
 	public async handleAuthCallback(code: string | null, state: string | null): Promise<void> {
 		this.ensureInitialized()
-		return this.authService!.handleCallback(code, state, (userInfo) => this.callbacks.onUserInfoChanged?.(userInfo))
+		return this.authService!.handleCallback(code, state)
 	}
 
 	// SettingsService
 
-	public getOrganizationSettings() {
+	public getOrganizationSettings(): OrganizationSettings | undefined {
 		this.ensureInitialized()
 		return this.settingsService!.getSettings()
 	}
 
-	public getAllowList() {
+	public getAllowList(): OrganizationAllowList {
 		this.ensureInitialized()
 		return this.settingsService!.getAllowList()
 	}
@@ -131,7 +120,6 @@ export class CloudService extends EventEmitter<CloudServiceEvents> {
 			this.settingsService.dispose()
 		}
 
-		this.removeAllListeners()
 		this.isInitialized = false
 	}
 

packages/cloud/src/__tests__/CloudService.test.ts

@@ -6,6 +6,7 @@ import { CloudService } from "../CloudService"
 import { AuthService } from "../AuthService"
 import { SettingsService } from "../SettingsService"
 import { TelemetryService } from "@roo-code/telemetry"
+import { CloudServiceCallbacks } from "../types"
 
 vi.mock("vscode", () => ({
 	ExtensionContext: vi.fn(),
@@ -108,16 +109,10 @@ describe("CloudService", () => {
 		}
 
 		vi.mocked(AuthService.createInstance).mockResolvedValue(mockAuthService as unknown as AuthService)
-		Object.defineProperty(AuthService, "instance", {
-			get: () => mockAuthService,
-			configurable: true,
-		})
+		Object.defineProperty(AuthService, "instance", { get: () => mockAuthService, configurable: true })
 
 		vi.mocked(SettingsService.createInstance).mockResolvedValue(mockSettingsService as unknown as SettingsService)
-		Object.defineProperty(SettingsService, "instance", {
-			get: () => mockSettingsService,
-			configurable: true,
-		})
+		Object.defineProperty(SettingsService, "instance", { get: () => mockSettingsService, configurable: true })
 
 		vi.mocked(TelemetryService.hasInstance).mockReturnValue(true)
 		Object.defineProperty(TelemetryService, "instance", {
@@ -133,11 +128,7 @@ describe("CloudService", () => {
 
 	describe("createInstance", () => {
 		it("should create and initialize CloudService instance", async () => {
-			const callbacks = {
-				onUserInfoChanged: vi.fn(),
-				onSettingsChanged: vi.fn(),
-			}
-
+			const callbacks = { userChanged: vi.fn(), settingsChanged: vi.fn() }
 			const cloudService = await CloudService.createInstance(mockContext, callbacks)
 
 			expect(cloudService).toBeInstanceOf(CloudService)
@@ -156,9 +147,11 @@ describe("CloudService", () => {
 
 	describe("authentication methods", () => {
 		let cloudService: CloudService
+		let callbacks: CloudServiceCallbacks
 
 		beforeEach(async () => {
-			cloudService = await CloudService.createInstance(mockContext)
+			callbacks = { userChanged: vi.fn(), settingsChanged: vi.fn() }
+			cloudService = await CloudService.createInstance(mockContext, callbacks)
 		})
 
 		it("should delegate login to AuthService", async () => {
@@ -168,7 +161,7 @@ describe("CloudService", () => {
 
 		it("should delegate logout to AuthService", async () => {
 			await cloudService.logout()
-			expect(mockAuthService.logout).toHaveBeenCalledWith(expect.any(Function))
+			expect(mockAuthService.logout).toHaveBeenCalled()
 		})
 
 		it("should delegate isAuthenticated to AuthService", () => {
@@ -201,7 +194,7 @@ describe("CloudService", () => {
 
 		it("should delegate handleAuthCallback to AuthService", async () => {
 			await cloudService.handleAuthCallback("code", "state")
-			expect(mockAuthService.handleCallback).toHaveBeenCalledWith("code", "state", expect.any(Function))
+			expect(mockAuthService.handleCallback).toHaveBeenCalledWith("code", "state")
 		})
 	})
 

packages/cloud/src/types.ts

@@ -0,0 +1,6 @@
+import { CloudUserInfo } from "@roo-code/types"
+
+export interface CloudServiceCallbacks {
+	userChanged?: (userInfo: CloudUserInfo | undefined) => void
+	settingsChanged?: () => void
+}

src/extension.ts

@@ -70,9 +70,9 @@ export async function activate(context: vscode.ExtensionContext) {
 
 	// Initialize Roo Code Cloud service.
 	await CloudService.createInstance(context, {
-		onUserInfoChanged: (userInfo) =>
+		userChanged: (userInfo) =>
 			ClineProvider.getVisibleInstance()?.postMessageToWebview({ type: "authenticatedUser", userInfo }),
-		onSettingsChanged: () => ClineProvider.getVisibleInstance()?.postStateToWebview(),
+		settingsChanged: () => ClineProvider.getVisibleInstance()?.postStateToWebview(),
 	})
 
 	// Initialize i18n for internationalization support