diff --git a/packages/cloud/src/CloudService.ts b/packages/cloud/src/CloudService.ts index 30d1545b23..ff33671a40 100644 --- a/packages/cloud/src/CloudService.ts +++ b/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 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,11 +70,8 @@ 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 @@ -69,14 +79,11 @@ export class CloudService { 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 { 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 } diff --git a/packages/cloud/src/CloudSettingsService.ts b/packages/cloud/src/CloudSettingsService.ts index 6692d8141d..4ce52774db 100644 --- a/packages/cloud/src/CloudSettingsService.ts +++ b/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 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,13 +56,16 @@ 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()) { @@ -63,7 +73,7 @@ export class CloudSettingsService implements SettingsService { } } - private async fetchSettings(callback: () => void): Promise { + private async fetchSettings(): Promise { 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() } } diff --git a/packages/cloud/src/__tests__/CloudService.test.ts b/packages/cloud/src/__tests__/CloudService.test.ts index 1384b6de6b..fd3ae9b9c0 100644 --- a/packages/cloud/src/__tests__/CloudService.test.ts +++ b/packages/cloud/src/__tests__/CloudService.test.ts @@ -9,7 +9,6 @@ import { CloudSettingsService } from "../CloudSettingsService" import { ShareService, TaskNotFoundError } from "../ShareService" import { TelemetryClient } from "../TelemetryClient" import { TelemetryService } from "@roo-code/telemetry" -import { CloudServiceCallbacks } from "../types" vi.mock("vscode", () => ({ ExtensionContext: vi.fn(), @@ -59,6 +58,8 @@ describe("CloudService", () => { getSettings: ReturnType getAllowList: ReturnType dispose: ReturnType + on: ReturnType + off: ReturnType } let mockShareService: { shareTask: ReturnType @@ -131,6 +132,8 @@ describe("CloudService", () => { getSettings: vi.fn(), getAllowList: vi.fn(), dispose: vi.fn(), + on: vi.fn(), + off: vi.fn(), } mockShareService = { @@ -168,20 +171,21 @@ describe("CloudService", () => { describe("createInstance", () => { it("should create and initialize CloudService instance", async () => { - const callbacks = { - stateChanged: vi.fn(), - } + const mockLog = vi.fn() - const cloudService = await CloudService.createInstance(mockContext, callbacks) + const cloudService = await CloudService.createInstance(mockContext, mockLog) expect(cloudService).toBeInstanceOf(CloudService) expect(WebAuthService).toHaveBeenCalledWith(mockContext, expect.any(Function)) - expect(CloudSettingsService).toHaveBeenCalledWith( - mockContext, - mockAuthService, - expect.any(Function), - expect.any(Function), - ) + expect(CloudSettingsService).toHaveBeenCalledWith(mockContext, mockAuthService, expect.any(Function)) + }) + + it("should set up event listeners for CloudSettingsService", async () => { + const mockLog = vi.fn() + + await CloudService.createInstance(mockContext, mockLog) + + expect(mockSettingsService.on).toHaveBeenCalledWith("settings-updated", expect.any(Function)) }) it("should throw error if instance already exists", async () => { @@ -195,11 +199,9 @@ describe("CloudService", () => { describe("authentication methods", () => { let cloudService: CloudService - let callbacks: CloudServiceCallbacks beforeEach(async () => { - callbacks = { stateChanged: vi.fn() } - cloudService = await CloudService.createInstance(mockContext, callbacks) + cloudService = await CloudService.createInstance(mockContext) }) it("should delegate login to AuthService", async () => { @@ -382,6 +384,105 @@ describe("CloudService", () => { expect(mockSettingsService.dispose).toHaveBeenCalled() }) + + it("should remove event listeners from CloudSettingsService", async () => { + // Create a mock that will pass the instanceof check + const mockCloudSettingsService = Object.create(CloudSettingsService.prototype) + Object.assign(mockCloudSettingsService, { + initialize: vi.fn(), + getSettings: vi.fn(), + getAllowList: vi.fn(), + dispose: vi.fn(), + on: vi.fn(), + off: vi.fn(), + }) + + // Override the mock to return our properly typed instance + vi.mocked(CloudSettingsService).mockImplementation(() => mockCloudSettingsService) + + const cloudService = await CloudService.createInstance(mockContext) + + // Verify the listener was added + expect(mockCloudSettingsService.on).toHaveBeenCalledWith("settings-updated", expect.any(Function)) + + // Get the listener function that was registered + const registeredListener = mockCloudSettingsService.on.mock.calls.find( + (call: unknown[]) => call[0] === "settings-updated", + )?.[1] + + cloudService.dispose() + + // Verify the listener was removed with the same function + expect(mockCloudSettingsService.off).toHaveBeenCalledWith("settings-updated", registeredListener) + }) + + it("should handle disposal when using StaticSettingsService", async () => { + // Reset the instance first + CloudService.resetInstance() + + // Mock a StaticSettingsService (which doesn't extend CloudSettingsService) + const mockStaticSettingsService = { + initialize: vi.fn(), + getSettings: vi.fn(), + getAllowList: vi.fn(), + dispose: vi.fn(), + on: vi.fn(), // Add on method to avoid initialization error + off: vi.fn(), // Add off method for disposal + } + + // Override the mock to return a service that won't pass instanceof check + vi.mocked(CloudSettingsService).mockImplementation( + () => mockStaticSettingsService as unknown as CloudSettingsService, + ) + + // This should not throw even though the service doesn't pass instanceof check + const _cloudService = await CloudService.createInstance(mockContext) + + // Should not throw when disposing + expect(() => _cloudService.dispose()).not.toThrow() + + // Should still call dispose on the settings service + expect(mockStaticSettingsService.dispose).toHaveBeenCalled() + // Should NOT call off method since it's not a CloudSettingsService instance + expect(mockStaticSettingsService.off).not.toHaveBeenCalled() + }) + }) + + describe("settings event handling", () => { + let _cloudService: CloudService + + beforeEach(async () => { + _cloudService = await CloudService.createInstance(mockContext) + }) + + it("should emit settings-updated event when settings are updated", async () => { + const settingsListener = vi.fn() + _cloudService.on("settings-updated", settingsListener) + + // Get the settings listener that was registered with the settings service + const serviceSettingsListener = mockSettingsService.on.mock.calls.find( + (call) => call[0] === "settings-updated", + )?.[1] + + expect(serviceSettingsListener).toBeDefined() + + // Simulate settings update event + const settingsData = { + settings: { + version: 2, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + }, + previousSettings: { + version: 1, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + }, + } + serviceSettingsListener(settingsData) + + expect(settingsListener).toHaveBeenCalledWith(settingsData) + }) }) describe("shareTask with ClineMessage retry logic", () => { @@ -397,7 +498,7 @@ describe("CloudService", () => { mockAuthService.hasOrIsAcquiringActiveSession.mockReturnValue(true) mockAuthService.getState.mockReturnValue("active") - cloudService = await CloudService.createInstance(mockContext, {}) + cloudService = await CloudService.createInstance(mockContext) }) it("should call shareTask without retry when successful", async () => { diff --git a/packages/cloud/src/__tests__/CloudSettingsService.test.ts b/packages/cloud/src/__tests__/CloudSettingsService.test.ts new file mode 100644 index 0000000000..e9d0ae3c93 --- /dev/null +++ b/packages/cloud/src/__tests__/CloudSettingsService.test.ts @@ -0,0 +1,476 @@ +import * as vscode from "vscode" +import { CloudSettingsService } from "../CloudSettingsService" +import { RefreshTimer } from "../RefreshTimer" +import type { AuthService } from "../auth" +import type { OrganizationSettings } from "@roo-code/types" + +// Mock dependencies +vi.mock("../RefreshTimer") +vi.mock("../Config", () => ({ + getRooCodeApiUrl: vi.fn().mockReturnValue("https://api.example.com"), +})) + +// Mock fetch globally +global.fetch = vi.fn() + +describe("CloudSettingsService", () => { + let mockContext: vscode.ExtensionContext + let mockAuthService: { + getState: ReturnType + getSessionToken: ReturnType + hasActiveSession: ReturnType + on: ReturnType + } + let mockRefreshTimer: { + start: ReturnType + stop: ReturnType + } + let cloudSettingsService: CloudSettingsService + let mockLog: ReturnType + + const mockSettings: OrganizationSettings = { + version: 1, + defaultSettings: {}, + allowList: { + allowAll: true, + providers: {}, + }, + } + + beforeEach(() => { + vi.clearAllMocks() + + mockContext = { + globalState: { + get: vi.fn(), + update: vi.fn().mockResolvedValue(undefined), + }, + } as unknown as vscode.ExtensionContext + + mockAuthService = { + getState: vi.fn().mockReturnValue("logged-out"), + getSessionToken: vi.fn(), + hasActiveSession: vi.fn().mockReturnValue(false), + on: vi.fn(), + } + + mockRefreshTimer = { + start: vi.fn(), + stop: vi.fn(), + } + + mockLog = vi.fn() + + // Mock RefreshTimer constructor + vi.mocked(RefreshTimer).mockImplementation(() => mockRefreshTimer as unknown as RefreshTimer) + + cloudSettingsService = new CloudSettingsService(mockContext, mockAuthService as unknown as AuthService, mockLog) + }) + + afterEach(() => { + cloudSettingsService.dispose() + }) + + describe("constructor", () => { + it("should create CloudSettingsService with proper dependencies", () => { + expect(cloudSettingsService).toBeInstanceOf(CloudSettingsService) + expect(RefreshTimer).toHaveBeenCalledWith({ + callback: expect.any(Function), + successInterval: 30000, + initialBackoffMs: 1000, + maxBackoffMs: 30000, + }) + }) + + it("should use console.log as default logger when none provided", () => { + const service = new CloudSettingsService(mockContext, mockAuthService as unknown as AuthService) + expect(service).toBeInstanceOf(CloudSettingsService) + }) + }) + + describe("initialize", () => { + it("should load cached settings on initialization", () => { + const cachedSettings = { + version: 1, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + } + + // Create a fresh mock context for this test + const testContext = { + globalState: { + get: vi.fn().mockReturnValue(cachedSettings), + update: vi.fn().mockResolvedValue(undefined), + }, + } as unknown as vscode.ExtensionContext + + // Mock auth service to not be logged out + const testAuthService = { + getState: vi.fn().mockReturnValue("active"), + getSessionToken: vi.fn(), + hasActiveSession: vi.fn().mockReturnValue(false), + on: vi.fn(), + } + + // Create a new instance to test initialization + const testService = new CloudSettingsService( + testContext, + testAuthService as unknown as AuthService, + mockLog, + ) + testService.initialize() + + expect(testContext.globalState.get).toHaveBeenCalledWith("organization-settings") + expect(testService.getSettings()).toEqual(cachedSettings) + + testService.dispose() + }) + + it("should clear cached settings if user is logged out", async () => { + const cachedSettings = { + version: 1, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + } + mockContext.globalState.get = vi.fn().mockReturnValue(cachedSettings) + mockAuthService.getState.mockReturnValue("logged-out") + + cloudSettingsService.initialize() + + expect(mockContext.globalState.update).toHaveBeenCalledWith("organization-settings", undefined) + }) + + it("should set up auth service event listeners", () => { + cloudSettingsService.initialize() + + expect(mockAuthService.on).toHaveBeenCalledWith("auth-state-changed", expect.any(Function)) + }) + + it("should start timer if user has active session", () => { + mockAuthService.hasActiveSession.mockReturnValue(true) + + cloudSettingsService.initialize() + + expect(mockRefreshTimer.start).toHaveBeenCalled() + }) + + it("should not start timer if user has no active session", () => { + mockAuthService.hasActiveSession.mockReturnValue(false) + + cloudSettingsService.initialize() + + expect(mockRefreshTimer.start).not.toHaveBeenCalled() + }) + }) + + describe("event emission", () => { + beforeEach(() => { + cloudSettingsService.initialize() + }) + + it("should emit 'settings-updated' event when settings change", async () => { + const eventSpy = vi.fn() + cloudSettingsService.on("settings-updated", eventSpy) + + mockAuthService.getSessionToken.mockReturnValue("valid-token") + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue(mockSettings), + } as unknown as Response) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + await timerCallback() + + expect(eventSpy).toHaveBeenCalledWith({ + settings: mockSettings, + previousSettings: undefined, + }) + }) + + it("should emit event with previous settings when updating existing settings", async () => { + const eventSpy = vi.fn() + + const previousSettings = { + version: 1, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + } + const newSettings = { + version: 2, + defaultSettings: {}, + allowList: { allowAll: true, providers: {} }, + } + + // Create a fresh mock context for this test + const testContext = { + globalState: { + get: vi.fn().mockReturnValue(previousSettings), + update: vi.fn().mockResolvedValue(undefined), + }, + } as unknown as vscode.ExtensionContext + + // Mock auth service to not be logged out + const testAuthService = { + getState: vi.fn().mockReturnValue("active"), + getSessionToken: vi.fn().mockReturnValue("valid-token"), + hasActiveSession: vi.fn().mockReturnValue(false), + on: vi.fn(), + } + + // Create a new service instance with cached settings + const testService = new CloudSettingsService( + testContext, + testAuthService as unknown as AuthService, + mockLog, + ) + testService.on("settings-updated", eventSpy) + testService.initialize() + + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue(newSettings), + } as unknown as Response) + + // Get the callback function passed to RefreshTimer for this instance + const timerCallback = + vi.mocked(RefreshTimer).mock.calls[vi.mocked(RefreshTimer).mock.calls.length - 1][0].callback + await timerCallback() + + expect(eventSpy).toHaveBeenCalledWith({ + settings: newSettings, + previousSettings, + }) + + testService.dispose() + }) + + it("should not emit event when settings version is unchanged", async () => { + const eventSpy = vi.fn() + + // Create a fresh mock context for this test + const testContext = { + globalState: { + get: vi.fn().mockReturnValue(mockSettings), + update: vi.fn().mockResolvedValue(undefined), + }, + } as unknown as vscode.ExtensionContext + + // Mock auth service to not be logged out + const testAuthService = { + getState: vi.fn().mockReturnValue("active"), + getSessionToken: vi.fn().mockReturnValue("valid-token"), + hasActiveSession: vi.fn().mockReturnValue(false), + on: vi.fn(), + } + + // Create a new service instance with cached settings + const testService = new CloudSettingsService( + testContext, + testAuthService as unknown as AuthService, + mockLog, + ) + testService.on("settings-updated", eventSpy) + testService.initialize() + + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue(mockSettings), // Same version + } as unknown as Response) + + // Get the callback function passed to RefreshTimer for this instance + const timerCallback = + vi.mocked(RefreshTimer).mock.calls[vi.mocked(RefreshTimer).mock.calls.length - 1][0].callback + await timerCallback() + + expect(eventSpy).not.toHaveBeenCalled() + + testService.dispose() + }) + + it("should not emit event when fetch fails", async () => { + const eventSpy = vi.fn() + cloudSettingsService.on("settings-updated", eventSpy) + + mockAuthService.getSessionToken.mockReturnValue("valid-token") + vi.mocked(fetch).mockResolvedValue({ + ok: false, + status: 500, + statusText: "Internal Server Error", + } as unknown as Response) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + await timerCallback() + + expect(eventSpy).not.toHaveBeenCalled() + }) + + it("should not emit event when no auth token available", async () => { + const eventSpy = vi.fn() + cloudSettingsService.on("settings-updated", eventSpy) + + mockAuthService.getSessionToken.mockReturnValue(null) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + await timerCallback() + + expect(eventSpy).not.toHaveBeenCalled() + expect(fetch).not.toHaveBeenCalled() + }) + }) + + describe("fetchSettings", () => { + beforeEach(() => { + cloudSettingsService.initialize() + }) + + it("should fetch and cache settings successfully", async () => { + mockAuthService.getSessionToken.mockReturnValue("valid-token") + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue(mockSettings), + } as unknown as Response) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + const result = await timerCallback() + + expect(result).toBe(true) + expect(fetch).toHaveBeenCalledWith("https://api.example.com/api/organization-settings", { + headers: { + Authorization: "Bearer valid-token", + }, + }) + expect(mockContext.globalState.update).toHaveBeenCalledWith("organization-settings", mockSettings) + }) + + it("should handle fetch errors gracefully", async () => { + mockAuthService.getSessionToken.mockReturnValue("valid-token") + vi.mocked(fetch).mockRejectedValue(new Error("Network error")) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + const result = await timerCallback() + + expect(result).toBe(false) + expect(mockLog).toHaveBeenCalledWith( + "[cloud-settings] Error fetching organization settings:", + expect.any(Error), + ) + }) + + it("should handle invalid response format", async () => { + mockAuthService.getSessionToken.mockReturnValue("valid-token") + vi.mocked(fetch).mockResolvedValue({ + ok: true, + json: vi.fn().mockResolvedValue({ invalid: "data" }), + } as unknown as Response) + + // Get the callback function passed to RefreshTimer + const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback + const result = await timerCallback() + + expect(result).toBe(false) + expect(mockLog).toHaveBeenCalledWith( + "[cloud-settings] Invalid organization settings format:", + expect.any(Object), + ) + }) + }) + + describe("getAllowList", () => { + it("should return settings allowList when available", () => { + mockContext.globalState.get = vi.fn().mockReturnValue(mockSettings) + cloudSettingsService.initialize() + + const allowList = cloudSettingsService.getAllowList() + expect(allowList).toEqual(mockSettings.allowList) + }) + + it("should return default allow all when no settings available", () => { + const allowList = cloudSettingsService.getAllowList() + expect(allowList).toEqual({ allowAll: true, providers: {} }) + }) + }) + + describe("getSettings", () => { + it("should return current settings", () => { + // Create a fresh mock context for this test + const testContext = { + globalState: { + get: vi.fn().mockReturnValue(mockSettings), + update: vi.fn().mockResolvedValue(undefined), + }, + } as unknown as vscode.ExtensionContext + + // Mock auth service to not be logged out + const testAuthService = { + getState: vi.fn().mockReturnValue("active"), + getSessionToken: vi.fn(), + hasActiveSession: vi.fn().mockReturnValue(false), + on: vi.fn(), + } + + const testService = new CloudSettingsService( + testContext, + testAuthService as unknown as AuthService, + mockLog, + ) + testService.initialize() + + const settings = testService.getSettings() + expect(settings).toEqual(mockSettings) + + testService.dispose() + }) + + it("should return undefined when no settings available", () => { + const settings = cloudSettingsService.getSettings() + expect(settings).toBeUndefined() + }) + }) + + describe("dispose", () => { + it("should remove all listeners and stop timer", () => { + const removeAllListenersSpy = vi.spyOn(cloudSettingsService, "removeAllListeners") + + cloudSettingsService.dispose() + + expect(removeAllListenersSpy).toHaveBeenCalled() + expect(mockRefreshTimer.stop).toHaveBeenCalled() + }) + }) + + describe("auth service event handlers", () => { + it("should start timer when auth-state-changed event is triggered with active-session", () => { + cloudSettingsService.initialize() + + // Get the auth-state-changed handler + const authStateChangedHandler = mockAuthService.on.mock.calls.find( + (call) => call[0] === "auth-state-changed", + )?.[1] + expect(authStateChangedHandler).toBeDefined() + + // Simulate active-session state change + authStateChangedHandler({ state: "active-session", previousState: "attempting-session" }) + expect(mockRefreshTimer.start).toHaveBeenCalled() + }) + + it("should stop timer and remove settings when auth-state-changed event is triggered with logged-out", async () => { + cloudSettingsService.initialize() + + // Get the auth-state-changed handler + const authStateChangedHandler = mockAuthService.on.mock.calls.find( + (call) => call[0] === "auth-state-changed", + )?.[1] + expect(authStateChangedHandler).toBeDefined() + + // Simulate logged-out state change from active-session + await authStateChangedHandler({ state: "logged-out", previousState: "active-session" }) + expect(mockRefreshTimer.stop).toHaveBeenCalled() + expect(mockContext.globalState.update).toHaveBeenCalledWith("organization-settings", undefined) + }) + }) +}) diff --git a/packages/cloud/src/__tests__/auth/StaticTokenAuthService.spec.ts b/packages/cloud/src/__tests__/auth/StaticTokenAuthService.spec.ts index cbf3a7b998..f1ab7f9abc 100644 --- a/packages/cloud/src/__tests__/auth/StaticTokenAuthService.spec.ts +++ b/packages/cloud/src/__tests__/auth/StaticTokenAuthService.spec.ts @@ -79,13 +79,13 @@ describe("StaticTokenAuthService", () => { expect(authService.getState()).toBe("active-session") }) - it("should emit active-session event on initialize", async () => { + it("should emit auth-state-changed event on initialize", async () => { const spy = vi.fn() - authService.on("active-session", spy) + authService.on("auth-state-changed", spy) await authService.initialize() - expect(spy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(spy).toHaveBeenCalledWith({ state: "active-session", previousState: "initializing" }) }) it("should log successful initialization", async () => { @@ -158,15 +158,15 @@ describe("StaticTokenAuthService", () => { describe("event emission", () => { it("should be able to register and emit events", async () => { - const activeSessionSpy = vi.fn() + const authStateChangedSpy = vi.fn() const userInfoSpy = vi.fn() - authService.on("active-session", activeSessionSpy) + authService.on("auth-state-changed", authStateChangedSpy) authService.on("user-info", userInfoSpy) await authService.initialize() - expect(activeSessionSpy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ state: "active-session", previousState: "initializing" }) // user-info event is not emitted in static token mode expect(userInfoSpy).not.toHaveBeenCalled() }) diff --git a/packages/cloud/src/__tests__/auth/WebAuthService.spec.ts b/packages/cloud/src/__tests__/auth/WebAuthService.spec.ts index 0e6681c20b..457e1d706d 100644 --- a/packages/cloud/src/__tests__/auth/WebAuthService.spec.ts +++ b/packages/cloud/src/__tests__/auth/WebAuthService.spec.ts @@ -165,34 +165,37 @@ describe("WebAuthService", () => { it("should transition to logged-out when no credentials exist", async () => { mockContext.secrets.get.mockResolvedValue(undefined) - const loggedOutSpy = vi.fn() - authService.on("logged-out", loggedOutSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await authService.initialize() expect(authService.getState()).toBe("logged-out") - expect(loggedOutSpy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ state: "logged-out", previousState: "initializing" }) }) it("should transition to attempting-session when valid credentials exist", async () => { const credentials = { clientToken: "test-token", sessionId: "test-session" } mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials)) - const attemptingSessionSpy = vi.fn() - authService.on("attempting-session", attemptingSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await authService.initialize() expect(authService.getState()).toBe("attempting-session") - expect(attemptingSessionSpy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "attempting-session", + previousState: "initializing", + }) expect(mockTimer.start).toHaveBeenCalled() }) it("should handle invalid credentials gracefully", async () => { mockContext.secrets.get.mockResolvedValue("invalid-json") - const loggedOutSpy = vi.fn() - authService.on("logged-out", loggedOutSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await authService.initialize() @@ -214,13 +217,13 @@ describe("WebAuthService", () => { const newCredentials = { clientToken: "new-token", sessionId: "new-session" } mockContext.secrets.get.mockResolvedValue(JSON.stringify(newCredentials)) - const attemptingSessionSpy = vi.fn() - authService.on("attempting-session", attemptingSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) onDidChangeCallback!({ key: "clerk-auth-credentials" }) await new Promise((resolve) => setTimeout(resolve, 0)) // Wait for async handling - expect(attemptingSessionSpy).toHaveBeenCalled() + expect(authStateChangedSpy).toHaveBeenCalled() }) }) @@ -344,13 +347,13 @@ describe("WebAuthService", () => { statusText: "Bad Request", }) - const loggedOutSpy = vi.fn() - authService.on("logged-out", loggedOutSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await expect(authService.handleCallback("auth-code", storedState)).rejects.toThrow( "Failed to handle Roo Code Cloud callback", ) - expect(loggedOutSpy).toHaveBeenCalled() + expect(authStateChangedSpy).toHaveBeenCalled() }) }) @@ -503,9 +506,9 @@ describe("WebAuthService", () => { }), }) - const activeSessionSpy = vi.fn() + const authStateChangedSpy = vi.fn() const userInfoSpy = vi.fn() - authService.on("active-session", activeSessionSpy) + authService.on("auth-state-changed", authStateChangedSpy) authService.on("user-info", userInfoSpy) // Trigger refresh by calling the timer callback @@ -518,7 +521,10 @@ describe("WebAuthService", () => { expect(authService.getState()).toBe("active-session") expect(authService.hasActiveSession()).toBe(true) expect(authService.getSessionToken()).toBe("new-jwt-token") - expect(activeSessionSpy).toHaveBeenCalledWith({ previousState: "attempting-session" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "active-session", + previousState: "attempting-session", + }) expect(userInfoSpy).toHaveBeenCalledWith({ userInfo: { name: "John Doe", @@ -560,8 +566,8 @@ describe("WebAuthService", () => { statusText: "Internal Server Error", }) - const inactiveSessionSpy = vi.fn() - authService.on("inactive-session", inactiveSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) // Verify we start in attempting-session state expect(authService.getState()).toBe("attempting-session") @@ -574,7 +580,10 @@ describe("WebAuthService", () => { // Should transition to inactive-session after first failure expect(authService.getState()).toBe("inactive-session") expect(authService["isFirstRefreshAttempt"]).toBe(false) - expect(inactiveSessionSpy).toHaveBeenCalledWith({ previousState: "attempting-session" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "inactive-session", + previousState: "attempting-session", + }) }) it("should not transition to inactive-session on subsequent failures", async () => { @@ -592,14 +601,14 @@ describe("WebAuthService", () => { expect(authService.getState()).toBe("inactive-session") expect(authService["isFirstRefreshAttempt"]).toBe(false) - const inactiveSessionSpy = vi.fn() - authService.on("inactive-session", inactiveSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) // Subsequent failure should not trigger another transition await expect(timerCallback()).rejects.toThrow() expect(authService.getState()).toBe("inactive-session") - expect(inactiveSessionSpy).not.toHaveBeenCalled() + expect(authStateChangedSpy).not.toHaveBeenCalled() }) it("should clear credentials on 401 during first refresh attempt (bug fix)", async () => { @@ -610,8 +619,8 @@ describe("WebAuthService", () => { statusText: "Unauthorized", }) - const loggedOutSpy = vi.fn() - authService.on("logged-out", loggedOutSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback await expect(timerCallback()).rejects.toThrow() @@ -625,7 +634,10 @@ describe("WebAuthService", () => { await authService["handleCredentialsChange"]() expect(authService.getState()).toBe("logged-out") - expect(loggedOutSpy).toHaveBeenCalledWith({ previousState: "attempting-session" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "logged-out", + previousState: "attempting-session", + }) }) }) @@ -788,28 +800,31 @@ describe("WebAuthService", () => { }) describe("event emissions", () => { - it("should emit logged-out event", async () => { - const loggedOutSpy = vi.fn() - authService.on("logged-out", loggedOutSpy) + it("should emit auth-state-changed event for logged-out", async () => { + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await authService.initialize() - expect(loggedOutSpy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ state: "logged-out", previousState: "initializing" }) }) - it("should emit attempting-session event", async () => { + it("should emit auth-state-changed event for attempting-session", async () => { const credentials = { clientToken: "test-token", sessionId: "test-session" } mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials)) - const attemptingSessionSpy = vi.fn() - authService.on("attempting-session", attemptingSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) await authService.initialize() - expect(attemptingSessionSpy).toHaveBeenCalledWith({ previousState: "initializing" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "attempting-session", + previousState: "initializing", + }) }) - it("should emit active-session event", async () => { + it("should emit auth-state-changed event for active-session", async () => { // Set up with credentials const credentials = { clientToken: "test-token", sessionId: "test-session" } mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials)) @@ -835,8 +850,8 @@ describe("WebAuthService", () => { }), }) - const activeSessionSpy = vi.fn() - authService.on("active-session", activeSessionSpy) + const authStateChangedSpy = vi.fn() + authService.on("auth-state-changed", authStateChangedSpy) const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback await timerCallback() @@ -844,7 +859,10 @@ describe("WebAuthService", () => { // Wait for async operations to complete await new Promise((resolve) => setTimeout(resolve, 0)) - expect(activeSessionSpy).toHaveBeenCalledWith({ previousState: "attempting-session" }) + expect(authStateChangedSpy).toHaveBeenCalledWith({ + state: "active-session", + previousState: "attempting-session", + }) }) it("should emit user-info event", async () => { @@ -1035,13 +1053,13 @@ describe("WebAuthService", () => { const newCredentials = { clientToken: "new-token", sessionId: "new-session" } mockContext.secrets.get.mockResolvedValue(JSON.stringify(newCredentials)) - const attemptingSessionSpy = vi.fn() - service.on("attempting-session", attemptingSessionSpy) + const authStateChangedSpy = vi.fn() + service.on("auth-state-changed", authStateChangedSpy) onDidChangeCallback!({ key: `clerk-auth-credentials-${customUrl}` }) await new Promise((resolve) => setTimeout(resolve, 0)) // Wait for async handling - expect(attemptingSessionSpy).toHaveBeenCalled() + expect(authStateChangedSpy).toHaveBeenCalled() }) it("should not respond to changes on different scoped keys", async () => { @@ -1058,14 +1076,14 @@ describe("WebAuthService", () => { const service = new WebAuthService(mockContext as unknown as vscode.ExtensionContext, mockLog) await service.initialize() - const inactiveSessionSpy = vi.fn() - service.on("inactive-session", inactiveSessionSpy) + const authStateChangedSpy = vi.fn() + service.on("auth-state-changed", authStateChangedSpy) // Simulate credentials change event with different scoped key onDidChangeCallback!({ key: "clerk-auth-credentials-https://other.clerk.com" }) await new Promise((resolve) => setTimeout(resolve, 0)) // Wait for async handling - expect(inactiveSessionSpy).not.toHaveBeenCalled() + expect(authStateChangedSpy).not.toHaveBeenCalled() }) it("should not respond to changes on default key when using scoped key", async () => { @@ -1082,14 +1100,14 @@ describe("WebAuthService", () => { const service = new WebAuthService(mockContext as unknown as vscode.ExtensionContext, mockLog) await service.initialize() - const inactiveSessionSpy = vi.fn() - service.on("inactive-session", inactiveSessionSpy) + const authStateChangedSpy = vi.fn() + service.on("auth-state-changed", authStateChangedSpy) // Simulate credentials change event with default key onDidChangeCallback!({ key: "clerk-auth-credentials" }) await new Promise((resolve) => setTimeout(resolve, 0)) // Wait for async handling - expect(inactiveSessionSpy).not.toHaveBeenCalled() + expect(authStateChangedSpy).not.toHaveBeenCalled() }) }) }) diff --git a/packages/cloud/src/auth/AuthService.ts b/packages/cloud/src/auth/AuthService.ts index 11ed5161ed..57e026d72a 100644 --- a/packages/cloud/src/auth/AuthService.ts +++ b/packages/cloud/src/auth/AuthService.ts @@ -2,10 +2,12 @@ import EventEmitter from "events" import type { CloudUserInfo } from "@roo-code/types" export interface AuthServiceEvents { - "attempting-session": [data: { previousState: AuthState }] - "inactive-session": [data: { previousState: AuthState }] - "active-session": [data: { previousState: AuthState }] - "logged-out": [data: { previousState: AuthState }] + "auth-state-changed": [ + data: { + state: AuthState + previousState: AuthState + }, + ] "user-info": [data: { userInfo: CloudUserInfo }] } diff --git a/packages/cloud/src/auth/StaticTokenAuthService.ts b/packages/cloud/src/auth/StaticTokenAuthService.ts index 11fc18d3fb..507f82c9f6 100644 --- a/packages/cloud/src/auth/StaticTokenAuthService.ts +++ b/packages/cloud/src/auth/StaticTokenAuthService.ts @@ -18,7 +18,7 @@ export class StaticTokenAuthService extends EventEmitter impl public async initialize(): Promise { const previousState: AuthState = "initializing" this.state = "active-session" - this.emit("active-session", { previousState }) + this.emit("auth-state-changed", { state: this.state, previousState }) this.log("[auth] Static token auth service initialized in active-session state") } diff --git a/packages/cloud/src/auth/WebAuthService.ts b/packages/cloud/src/auth/WebAuthService.ts index 82d3122426..8fd892f44f 100644 --- a/packages/cloud/src/auth/WebAuthService.ts +++ b/packages/cloud/src/auth/WebAuthService.ts @@ -113,6 +113,12 @@ export class WebAuthService extends EventEmitter implements A }) } + private changeState(newState: AuthState): void { + const previousState = this.state + this.state = newState + this.emit("auth-state-changed", { state: newState, previousState }) + } + private async handleCredentialsChange(): Promise { try { const credentials = await this.loadCredentials() @@ -138,14 +144,11 @@ export class WebAuthService extends EventEmitter implements A private transitionToLoggedOut(): void { this.timer.stop() - const previousState = this.state - this.credentials = null this.sessionToken = null this.userInfo = null - this.state = "logged-out" - this.emit("logged-out", { previousState }) + this.changeState("logged-out") this.log("[auth] Transitioned to logged-out state") } @@ -153,14 +156,11 @@ export class WebAuthService extends EventEmitter implements A private transitionToAttemptingSession(credentials: AuthCredentials): void { this.credentials = credentials - const previousState = this.state - this.state = "attempting-session" - this.sessionToken = null this.userInfo = null this.isFirstRefreshAttempt = true - this.emit("attempting-session", { previousState }) + this.changeState("attempting-session") this.timer.start() @@ -168,13 +168,10 @@ export class WebAuthService extends EventEmitter implements A } private transitionToInactiveSession(): void { - const previousState = this.state - this.state = "inactive-session" - this.sessionToken = null this.userInfo = null - this.emit("inactive-session", { previousState }) + this.changeState("inactive-session") this.log("[auth] Transitioned to inactive-session state") } @@ -302,9 +299,7 @@ export class WebAuthService extends EventEmitter implements A this.log("[auth] Successfully authenticated with Roo Code Cloud") } catch (error) { this.log(`[auth] Error handling Roo Code Cloud callback: ${error}`) - const previousState = this.state - this.state = "logged-out" - this.emit("logged-out", { previousState }) + this.changeState("logged-out") throw new Error(`Failed to handle Roo Code Cloud callback: ${error}`) } } @@ -388,12 +383,13 @@ export class WebAuthService extends EventEmitter implements A try { const previousState = this.state this.sessionToken = await this.clerkCreateSessionToken() - this.state = "active-session" if (previousState !== "active-session") { + this.changeState("active-session") this.log("[auth] Transitioned to active-session state") - this.emit("active-session", { previousState }) this.fetchUserInfo() + } else { + this.state = "active-session" } } catch (error) { if (error instanceof InvalidClientTokenError) { diff --git a/packages/cloud/src/types.ts b/packages/cloud/src/types.ts index 0139bb78ec..78275b32e2 100644 --- a/packages/cloud/src/types.ts +++ b/packages/cloud/src/types.ts @@ -1,4 +1,4 @@ -export interface CloudServiceCallbacks { - stateChanged?: () => void - log?: (...args: unknown[]) => void -} +import { AuthServiceEvents } from "./auth" +import { SettingsServiceEvents } from "./CloudSettingsService" + +export type CloudServiceEvents = AuthServiceEvents & SettingsServiceEvents diff --git a/src/extension.ts b/src/extension.ts index bd43bcbf8a..60c61aada7 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -75,10 +75,15 @@ export async function activate(context: vscode.ExtensionContext) { const cloudLogger = createDualLogger(createOutputChannelLogger(outputChannel)) // Initialize Roo Code Cloud service. - await CloudService.createInstance(context, { - stateChanged: () => ClineProvider.getVisibleInstance()?.postStateToWebview(), - log: cloudLogger, - }) + const cloudService = await CloudService.createInstance(context, cloudLogger) + const postStateListener = () => { + ClineProvider.getVisibleInstance()?.postStateToWebview() + } + cloudService.on("auth-state-changed", postStateListener) + cloudService.on("user-info", postStateListener) + cloudService.on("settings-updated", postStateListener) + // Add to subscriptions for proper cleanup on deactivate + context.subscriptions.push(cloudService) // Initialize MDM service const mdmService = await MdmService.createInstance(cloudLogger)