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>
This commit is contained in:
John Richmond 2025-07-31 15:30:30 -07:00 committed by GitHub
parent 079fc22a66
commit 6e835de82f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 764 additions and 143 deletions

View file

@ -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,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<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
}

View file

@ -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,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<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()
}
}

View file

@ -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<typeof vi.fn>
getAllowList: ReturnType<typeof vi.fn>
dispose: ReturnType<typeof vi.fn>
on: ReturnType<typeof vi.fn>
off: ReturnType<typeof vi.fn>
}
let mockShareService: {
shareTask: ReturnType<typeof vi.fn>
@ -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 () => {

View file

@ -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<typeof vi.fn>
getSessionToken: ReturnType<typeof vi.fn>
hasActiveSession: ReturnType<typeof vi.fn>
on: ReturnType<typeof vi.fn>
}
let mockRefreshTimer: {
start: ReturnType<typeof vi.fn>
stop: ReturnType<typeof vi.fn>
}
let cloudSettingsService: CloudSettingsService
let mockLog: ReturnType<typeof vi.fn>
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)
})
})
})

View file

@ -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()
})

View file

@ -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()
})
})
})

View file

@ -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 }]
}

View file

@ -18,7 +18,7 @@ export class StaticTokenAuthService extends EventEmitter<AuthServiceEvents> impl
public async initialize(): Promise<void> {
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")
}

View file

@ -113,6 +113,12 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> 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<void> {
try {
const credentials = await this.loadCredentials()
@ -138,14 +144,11 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> 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<AuthServiceEvents> 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<AuthServiceEvents> 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<AuthServiceEvents> 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<AuthServiceEvents> 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) {

View file

@ -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

View file

@ -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)