Browse Source

cloud service cleanup (auth mostly) (#4539)

John Richmond 8 months ago
parent
commit
a879f247f3

+ 114 - 80
packages/cloud/src/AuthService.ts

@@ -1,7 +1,6 @@
 import crypto from "crypto"
 import EventEmitter from "events"
 
-import axios from "axios"
 import * as vscode from "vscode"
 import { z } from "zod"
 
@@ -30,6 +29,61 @@ const AUTH_STATE_KEY = "clerk-auth-state"
 
 type AuthState = "initializing" | "logged-out" | "active-session" | "inactive-session"
 
+const clerkSignInResponseSchema = z.object({
+	response: z.object({
+		created_session_id: z.string(),
+	}),
+})
+
+const clerkCreateSessionTokenResponseSchema = z.object({
+	jwt: z.string(),
+})
+
+const clerkMeResponseSchema = z.object({
+	response: z.object({
+		first_name: z.string().optional(),
+		last_name: z.string().optional(),
+		image_url: z.string().optional(),
+		primary_email_address_id: z.string().optional(),
+		email_addresses: z
+			.array(
+				z.object({
+					id: z.string(),
+					email_address: z.string(),
+				}),
+			)
+			.optional(),
+	}),
+})
+
+const clerkOrganizationMembershipsSchema = z.object({
+	response: z.array(
+		z.object({
+			id: z.string(),
+			role: z.string(),
+			permissions: z.array(z.string()).optional(),
+			created_at: z.number().optional(),
+			updated_at: z.number().optional(),
+			organization: z.object({
+				id: z.string(),
+				name: z.string(),
+				slug: z.string().optional(),
+				image_url: z.string().optional(),
+				has_image: z.boolean().optional(),
+				created_at: z.number().optional(),
+				updated_at: z.number().optional(),
+			}),
+		}),
+	),
+})
+
+class InvalidClientTokenError extends Error {
+	constructor() {
+		super("Invalid/Expired client token")
+		Object.setPrototypeOf(this, InvalidClientTokenError.prototype)
+	}
+}
+
 export class AuthService extends EventEmitter<AuthServiceEvents> {
 	private context: vscode.ExtensionContext
 	private timer: RefreshTimer
@@ -208,7 +262,7 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 				throw new Error("Invalid state parameter. Authentication request may have been tampered with.")
 			}
 
-			const { credentials } = await this.clerkSignIn(code)
+			const credentials = await this.clerkSignIn(code)
 
 			await this.storeCredentials(credentials)
 
@@ -285,7 +339,6 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	private async refreshSession(): Promise<void> {
 		if (!this.credentials) {
 			this.log("[auth] Cannot refresh session: missing credentials")
-			this.state = "inactive-session"
 			return
 		}
 
@@ -300,6 +353,10 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 				this.fetchUserInfo()
 			}
 		} catch (error) {
+			if (error instanceof InvalidClientTokenError) {
+				this.log("[auth] Invalid/Expired client token: clearing credentials")
+				this.clearCredentials()
+			}
 			this.log("[auth] Failed to refresh session", error)
 			throw error
 		}
@@ -323,103 +380,93 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 		return this.userInfo
 	}
 
-	private async clerkSignIn(ticket: string): Promise<{ credentials: AuthCredentials; sessionToken: string }> {
+	private async clerkSignIn(ticket: string): Promise<AuthCredentials> {
 		const formData = new URLSearchParams()
 		formData.append("strategy", "ticket")
 		formData.append("ticket", ticket)
 
-		const response = await axios.post(`${getClerkBaseUrl()}/v1/client/sign_ins`, formData, {
+		const response = await fetch(`${getClerkBaseUrl()}/v1/client/sign_ins`, {
+			method: "POST",
 			headers: {
 				"Content-Type": "application/x-www-form-urlencoded",
 				"User-Agent": this.userAgent(),
 			},
+			body: formData.toString(),
+			signal: AbortSignal.timeout(10000),
 		})
 
-		// 3. Extract the client token from the Authorization header.
-		const clientToken = response.headers.authorization
-
-		if (!clientToken) {
-			throw new Error("No authorization header found in the response")
-		}
-
-		// 4. Find the session using created_session_id and extract the JWT.
-		const sessionId = response.data?.response?.created_session_id
-
-		if (!sessionId) {
-			throw new Error("No session ID found in the response")
+		if (!response.ok) {
+			throw new Error(`HTTP ${response.status}: ${response.statusText}`)
 		}
 
-		// Find the session in the client sessions array.
-		const session = response.data?.client?.sessions?.find((s: { id: string }) => s.id === sessionId)
-
-		if (!session) {
-			throw new Error("Session not found in the response")
-		}
+		const {
+			response: { created_session_id: sessionId },
+		} = clerkSignInResponseSchema.parse(await response.json())
 
-		// Extract the session token (JWT) and store it.
-		const sessionToken = session.last_active_token?.jwt
+		// 3. Extract the client token from the Authorization header.
+		const clientToken = response.headers.get("authorization")
 
-		if (!sessionToken) {
-			throw new Error("Session does not have a token")
+		if (!clientToken) {
+			throw new Error("No authorization header found in the response")
 		}
 
-		const credentials = authCredentialsSchema.parse({ clientToken, sessionId })
-
-		return { credentials, sessionToken }
+		return authCredentialsSchema.parse({ clientToken, sessionId })
 	}
 
 	private async clerkCreateSessionToken(): Promise<string> {
 		const formData = new URLSearchParams()
 		formData.append("_is_native", "1")
 
-		const response = await axios.post(
-			`${getClerkBaseUrl()}/v1/client/sessions/${this.credentials!.sessionId}/tokens`,
-			formData,
-			{
-				headers: {
-					"Content-Type": "application/x-www-form-urlencoded",
-					Authorization: `Bearer ${this.credentials!.clientToken}`,
-					"User-Agent": this.userAgent(),
-				},
+		const response = await fetch(`${getClerkBaseUrl()}/v1/client/sessions/${this.credentials!.sessionId}/tokens`, {
+			method: "POST",
+			headers: {
+				"Content-Type": "application/x-www-form-urlencoded",
+				Authorization: `Bearer ${this.credentials!.clientToken}`,
+				"User-Agent": this.userAgent(),
 			},
-		)
-
-		const sessionToken = response.data?.jwt
+			body: formData.toString(),
+			signal: AbortSignal.timeout(10000),
+		})
 
-		if (!sessionToken) {
-			throw new Error("No JWT found in refresh response")
+		if (response.status >= 400 && response.status < 500) {
+			throw new InvalidClientTokenError()
+		} else if (!response.ok) {
+			throw new Error(`HTTP ${response.status}: ${response.statusText}`)
 		}
 
-		return sessionToken
+		const data = clerkCreateSessionTokenResponseSchema.parse(await response.json())
+
+		return data.jwt
 	}
 
 	private async clerkMe(): Promise<CloudUserInfo> {
-		const response = await axios.get(`${getClerkBaseUrl()}/v1/me`, {
+		const response = await fetch(`${getClerkBaseUrl()}/v1/me`, {
 			headers: {
 				Authorization: `Bearer ${this.credentials!.clientToken}`,
 				"User-Agent": this.userAgent(),
 			},
+			signal: AbortSignal.timeout(10000),
 		})
 
-		const userData = response.data?.response
-
-		if (!userData) {
-			throw new Error("No response user data")
+		if (!response.ok) {
+			throw new Error(`HTTP ${response.status}: ${response.statusText}`)
 		}
 
+		const { response: userData } = clerkMeResponseSchema.parse(await response.json())
+
 		const userInfo: CloudUserInfo = {}
 
-		userInfo.name = `${userData?.first_name} ${userData?.last_name}`
-		const primaryEmailAddressId = userData?.primary_email_address_id
-		const emailAddresses = userData?.email_addresses
+		userInfo.name = `${userData.first_name} ${userData.last_name}`
+		const primaryEmailAddressId = userData.primary_email_address_id
+		const emailAddresses = userData.email_addresses
 
 		if (primaryEmailAddressId && emailAddresses) {
 			userInfo.email = emailAddresses.find(
-				(email: { id: string }) => primaryEmailAddressId === email?.id,
+				(email: { id: string }) => primaryEmailAddressId === email.id,
 			)?.email_address
 		}
 
-		userInfo.picture = userData?.image_url
+		userInfo.picture = userData.image_url
 
 		// Fetch organization memberships separately
 		try {
@@ -444,51 +491,38 @@ export class AuthService extends EventEmitter<AuthServiceEvents> {
 	}
 
 	private async clerkGetOrganizationMemberships(): Promise<CloudOrganizationMembership[]> {
-		const response = await axios.get(`${getClerkBaseUrl()}/v1/me/organization_memberships`, {
+		const response = await fetch(`${getClerkBaseUrl()}/v1/me/organization_memberships`, {
 			headers: {
 				Authorization: `Bearer ${this.credentials!.clientToken}`,
 				"User-Agent": this.userAgent(),
 			},
+			signal: AbortSignal.timeout(10000),
 		})
 
-		// The response structure is: { response: [...] }
-		// Extract the organization memberships from the response.response array
-		return response.data?.response || []
+		return clerkOrganizationMembershipsSchema.parse(await response.json()).response
 	}
 
 	private async clerkLogout(credentials: AuthCredentials): Promise<void> {
 		const formData = new URLSearchParams()
 		formData.append("_is_native", "1")
 
-		await axios.post(`${getClerkBaseUrl()}/v1/client/sessions/${credentials.sessionId}/remove`, formData, {
+		const response = await fetch(`${getClerkBaseUrl()}/v1/client/sessions/${credentials.sessionId}/remove`, {
+			method: "POST",
 			headers: {
+				"Content-Type": "application/x-www-form-urlencoded",
 				Authorization: `Bearer ${credentials.clientToken}`,
 				"User-Agent": this.userAgent(),
 			},
+			body: formData.toString(),
+			signal: AbortSignal.timeout(10000),
 		})
-	}
-
-	private userAgent(): string {
-		return getUserAgent(this.context)
-	}
 
-	private static _instance: AuthService | null = null
-
-	static get instance() {
-		if (!this._instance) {
-			throw new Error("AuthService not initialized")
+		if (!response.ok) {
+			throw new Error(`HTTP ${response.status}: ${response.statusText}`)
 		}
-
-		return this._instance
 	}
 
-	static async createInstance(context: vscode.ExtensionContext, log?: (...args: unknown[]) => void) {
-		if (this._instance) {
-			throw new Error("AuthService instance already created")
-		}
-
-		this._instance = new AuthService(context, log)
-		await this._instance.initialize()
-		return this._instance
+	private userAgent(): string {
+		return getUserAgent(this.context)
 	}
 }

+ 5 - 9
packages/cloud/src/CloudService.ts

@@ -37,16 +37,18 @@ export class CloudService {
 		}
 
 		try {
-			this.authService = await AuthService.createInstance(this.context, this.log)
+			this.authService = new AuthService(this.context, this.log)
+			await this.authService.initialize()
 
 			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.settingsService = await SettingsService.createInstance(this.context, () =>
+			this.settingsService = new SettingsService(this.context, this.authService, () =>
 				this.callbacks.stateChanged?.(),
 			)
+			this.settingsService.initialize()
 
 			this.telemetryClient = new TelemetryClient(this.authService, this.settingsService)
 
@@ -162,13 +164,7 @@ export class CloudService {
 	}
 
 	private ensureInitialized(): void {
-		if (
-			!this.isInitialized ||
-			!this.authService ||
-			!this.settingsService ||
-			!this.telemetryClient ||
-			!this.shareService
-		) {
+		if (!this.isInitialized) {
 			throw new Error("CloudService not initialized.")
 		}
 	}

+ 1 - 19
packages/cloud/src/SettingsService.ts

@@ -14,14 +14,13 @@ import { RefreshTimer } from "./RefreshTimer"
 const ORGANIZATION_SETTINGS_CACHE_KEY = "organization-settings"
 
 export class SettingsService {
-	private static _instance: SettingsService | null = null
 
 	private context: vscode.ExtensionContext
 	private authService: AuthService
 	private settings: OrganizationSettings | undefined = undefined
 	private timer: RefreshTimer
 
-	private constructor(context: vscode.ExtensionContext, authService: AuthService, callback: () => void) {
+	constructor(context: vscode.ExtensionContext, authService: AuthService, callback: () => void) {
 		this.context = context
 		this.authService = authService
 
@@ -122,21 +121,4 @@ export class SettingsService {
 		this.timer.stop()
 	}
 
-	static get instance() {
-		if (!this._instance) {
-			throw new Error("SettingsService not initialized")
-		}
-
-		return this._instance
-	}
-
-	static async createInstance(context: vscode.ExtensionContext, callback: () => void) {
-		if (this._instance) {
-			throw new Error("SettingsService instance already created")
-		}
-
-		this._instance = new SettingsService(context, AuthService.instance, callback)
-		this._instance.initialize()
-		return this._instance
-	}
 }

+ 8 - 0
packages/cloud/src/__mocks__/vscode.ts

@@ -18,14 +18,18 @@ export interface ExtensionContext {
 		get: (key: string) => Promise<string | undefined>
 		store: (key: string, value: string) => Promise<void>
 		delete: (key: string) => Promise<void>
+		onDidChange: (listener: (e: { key: string }) => void) => { dispose: () => void }
 	}
 	globalState: {
 		get: <T>(key: string) => T | undefined
 		update: (key: string, value: any) => Promise<void>
 	}
+	subscriptions: any[]
 	extension?: {
 		packageJSON?: {
 			version?: string
+			publisher?: string
+			name?: string
 		}
 	}
 }
@@ -36,14 +40,18 @@ export const mockExtensionContext: ExtensionContext = {
 		get: vi.fn().mockResolvedValue(undefined),
 		store: vi.fn().mockResolvedValue(undefined),
 		delete: vi.fn().mockResolvedValue(undefined),
+		onDidChange: vi.fn().mockReturnValue({ dispose: vi.fn() }),
 	},
 	globalState: {
 		get: vi.fn().mockReturnValue(undefined),
 		update: vi.fn().mockResolvedValue(undefined),
 	},
+	subscriptions: [],
 	extension: {
 		packageJSON: {
 			version: "1.0.0",
+			publisher: "RooVeterinaryInc",
+			name: "roo-cline",
 		},
 	},
 }

+ 815 - 0
packages/cloud/src/__tests__/AuthService.spec.ts

@@ -0,0 +1,815 @@
+// npx vitest run src/__tests__/AuthService.spec.ts
+
+import { vi, Mock, beforeEach, afterEach, describe, it, expect } from "vitest"
+import crypto from "crypto"
+import * as vscode from "vscode"
+
+import { AuthService } from "../AuthService"
+import { RefreshTimer } from "../RefreshTimer"
+import * as Config from "../Config"
+import * as utils from "../utils"
+
+// Mock external dependencies
+vi.mock("../RefreshTimer")
+vi.mock("../Config")
+vi.mock("../utils")
+vi.mock("crypto")
+
+// Mock fetch globally
+const mockFetch = vi.fn()
+global.fetch = mockFetch
+
+// Mock vscode module
+vi.mock("vscode", () => ({
+	window: {
+		showInformationMessage: vi.fn(),
+		showErrorMessage: vi.fn(),
+	},
+	env: {
+		openExternal: vi.fn(),
+		uriScheme: "vscode",
+	},
+	Uri: {
+		parse: vi.fn((uri: string) => ({ toString: () => uri })),
+	},
+}))
+
+describe("AuthService", () => {
+	let authService: AuthService
+	let mockTimer: {
+		start: Mock
+		stop: Mock
+		reset: Mock
+	}
+	let mockLog: Mock
+	let mockContext: {
+		subscriptions: { push: Mock }
+		secrets: {
+			get: Mock
+			store: Mock
+			delete: Mock
+			onDidChange: Mock
+		}
+		globalState: {
+			get: Mock
+			update: Mock
+		}
+		extension: {
+			packageJSON: {
+				version: string
+				publisher: string
+				name: string
+			}
+		}
+	}
+
+	beforeEach(() => {
+		// Reset all mocks
+		vi.clearAllMocks()
+
+		// Setup mock context with proper subscriptions array
+		mockContext = {
+			subscriptions: {
+				push: vi.fn(),
+			},
+			secrets: {
+				get: vi.fn().mockResolvedValue(undefined),
+				store: vi.fn().mockResolvedValue(undefined),
+				delete: vi.fn().mockResolvedValue(undefined),
+				onDidChange: vi.fn().mockReturnValue({ dispose: vi.fn() }),
+			},
+			globalState: {
+				get: vi.fn().mockReturnValue(undefined),
+				update: vi.fn().mockResolvedValue(undefined),
+			},
+			extension: {
+				packageJSON: {
+					version: "1.0.0",
+					publisher: "RooVeterinaryInc",
+					name: "roo-cline",
+				},
+			},
+		}
+
+		// Setup timer mock
+		mockTimer = {
+			start: vi.fn(),
+			stop: vi.fn(),
+			reset: vi.fn(),
+		}
+		vi.mocked(RefreshTimer).mockImplementation(() => mockTimer as unknown as RefreshTimer)
+
+		// Setup config mocks
+		vi.mocked(Config.getClerkBaseUrl).mockReturnValue("https://clerk.test.com")
+		vi.mocked(Config.getRooCodeApiUrl).mockReturnValue("https://api.test.com")
+
+		// Setup utils mock
+		vi.mocked(utils.getUserAgent).mockReturnValue("Roo-Code 1.0.0")
+
+		// Setup crypto mock
+		vi.mocked(crypto.randomBytes).mockReturnValue(Buffer.from("test-random-bytes") as never)
+
+		// Setup log mock
+		mockLog = vi.fn()
+
+		authService = new AuthService(mockContext as unknown as vscode.ExtensionContext, mockLog)
+	})
+
+	afterEach(() => {
+		vi.clearAllMocks()
+	})
+
+	describe("constructor", () => {
+		it("should initialize with correct default values", () => {
+			expect(authService.getState()).toBe("initializing")
+			expect(authService.isAuthenticated()).toBe(false)
+			expect(authService.hasActiveSession()).toBe(false)
+			expect(authService.getSessionToken()).toBeUndefined()
+			expect(authService.getUserInfo()).toBeNull()
+		})
+
+		it("should create RefreshTimer with correct configuration", () => {
+			expect(RefreshTimer).toHaveBeenCalledWith({
+				callback: expect.any(Function),
+				successInterval: 50_000,
+				initialBackoffMs: 1_000,
+				maxBackoffMs: 300_000,
+			})
+		})
+
+		it("should use console.log as default logger", () => {
+			const serviceWithoutLog = new AuthService(mockContext as unknown as vscode.ExtensionContext)
+			// Can't directly test console.log usage, but constructor should not throw
+			expect(serviceWithoutLog).toBeInstanceOf(AuthService)
+		})
+	})
+
+	describe("initialize", () => {
+		it("should handle credentials change and setup event listener", async () => {
+			await authService.initialize()
+
+			expect(mockContext.subscriptions.push).toHaveBeenCalled()
+			expect(mockContext.secrets.onDidChange).toHaveBeenCalled()
+		})
+
+		it("should not initialize twice", async () => {
+			await authService.initialize()
+			const firstCallCount = vi.mocked(mockContext.secrets.onDidChange).mock.calls.length
+
+			await authService.initialize()
+			expect(mockContext.secrets.onDidChange).toHaveBeenCalledTimes(firstCallCount)
+			expect(mockLog).toHaveBeenCalledWith("[auth] initialize() called after already initialized")
+		})
+
+		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)
+
+			await authService.initialize()
+
+			expect(authService.getState()).toBe("logged-out")
+			expect(loggedOutSpy).toHaveBeenCalledWith({ previousState: "initializing" })
+		})
+
+		it("should transition to inactive-session when valid credentials exist", async () => {
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+
+			const inactiveSessionSpy = vi.fn()
+			authService.on("inactive-session", inactiveSessionSpy)
+
+			await authService.initialize()
+
+			expect(authService.getState()).toBe("inactive-session")
+			expect(inactiveSessionSpy).toHaveBeenCalledWith({ 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)
+
+			await authService.initialize()
+
+			expect(authService.getState()).toBe("logged-out")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Failed to parse stored credentials:", expect.any(Error))
+		})
+
+		it("should handle credentials change events", async () => {
+			let onDidChangeCallback: (e: { key: string }) => void
+
+			mockContext.secrets.onDidChange.mockImplementation((callback: (e: { key: string }) => void) => {
+				onDidChangeCallback = callback
+				return { dispose: vi.fn() }
+			})
+
+			await authService.initialize()
+
+			// Simulate credentials change event
+			const newCredentials = { clientToken: "new-token", sessionId: "new-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(newCredentials))
+
+			const inactiveSessionSpy = vi.fn()
+			authService.on("inactive-session", inactiveSessionSpy)
+
+			onDidChangeCallback!({ key: "clerk-auth-credentials" })
+			await new Promise((resolve) => setTimeout(resolve, 0)) // Wait for async handling
+
+			expect(inactiveSessionSpy).toHaveBeenCalled()
+		})
+	})
+
+	describe("login", () => {
+		beforeEach(async () => {
+			await authService.initialize()
+		})
+
+		it("should generate state and open external URL", async () => {
+			const mockOpenExternal = vi.fn()
+			const vscode = await import("vscode")
+			vi.mocked(vscode.env.openExternal).mockImplementation(mockOpenExternal)
+
+			await authService.login()
+
+			expect(crypto.randomBytes).toHaveBeenCalledWith(16)
+			expect(mockContext.globalState.update).toHaveBeenCalledWith(
+				"clerk-auth-state",
+				"746573742d72616e646f6d2d6279746573",
+			)
+			expect(mockOpenExternal).toHaveBeenCalledWith(
+				expect.objectContaining({
+					toString: expect.any(Function),
+				}),
+			)
+		})
+
+		it("should use package.json values for redirect URI", async () => {
+			const mockOpenExternal = vi.fn()
+			const vscode = await import("vscode")
+			vi.mocked(vscode.env.openExternal).mockImplementation(mockOpenExternal)
+
+			await authService.login()
+
+			const expectedUrl =
+				"https://api.test.com/extension/sign-in?state=746573742d72616e646f6d2d6279746573&auth_redirect=vscode%3A%2F%2FRooVeterinaryInc.roo-cline"
+			expect(mockOpenExternal).toHaveBeenCalledWith(
+				expect.objectContaining({
+					toString: expect.any(Function),
+				}),
+			)
+
+			// Verify the actual URL
+			const calledUri = mockOpenExternal.mock.calls[0][0]
+			expect(calledUri.toString()).toBe(expectedUrl)
+		})
+
+		it("should handle errors during login", async () => {
+			vi.mocked(crypto.randomBytes).mockImplementation(() => {
+				throw new Error("Crypto error")
+			})
+
+			await expect(authService.login()).rejects.toThrow("Failed to initiate Roo Code Cloud authentication")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Error initiating Roo Code Cloud auth: Error: Crypto error")
+		})
+	})
+
+	describe("handleCallback", () => {
+		beforeEach(async () => {
+			await authService.initialize()
+		})
+
+		it("should handle invalid parameters", async () => {
+			const vscode = await import("vscode")
+			const mockShowInfo = vi.fn()
+			vi.mocked(vscode.window.showInformationMessage).mockImplementation(mockShowInfo)
+
+			await authService.handleCallback(null, "state")
+			expect(mockShowInfo).toHaveBeenCalledWith("Invalid Roo Code Cloud sign in url")
+
+			await authService.handleCallback("code", null)
+			expect(mockShowInfo).toHaveBeenCalledWith("Invalid Roo Code Cloud sign in url")
+		})
+
+		it("should validate state parameter", async () => {
+			mockContext.globalState.get.mockReturnValue("stored-state")
+
+			await expect(authService.handleCallback("code", "different-state")).rejects.toThrow(
+				"Failed to handle Roo Code Cloud callback",
+			)
+			expect(mockLog).toHaveBeenCalledWith("[auth] State mismatch in callback")
+		})
+
+		it("should successfully handle valid callback", async () => {
+			const storedState = "valid-state"
+			mockContext.globalState.get.mockReturnValue(storedState)
+
+			// Mock successful Clerk sign-in response
+			const mockResponse = {
+				ok: true,
+				json: () =>
+					Promise.resolve({
+						response: { created_session_id: "session-123" },
+					}),
+				headers: {
+					get: (header: string) => (header === "authorization" ? "Bearer token-123" : null),
+				},
+			}
+			mockFetch.mockResolvedValue(mockResponse)
+
+			const vscode = await import("vscode")
+			const mockShowInfo = vi.fn()
+			vi.mocked(vscode.window.showInformationMessage).mockImplementation(mockShowInfo)
+
+			await authService.handleCallback("auth-code", storedState)
+
+			expect(mockContext.secrets.store).toHaveBeenCalledWith(
+				"clerk-auth-credentials",
+				JSON.stringify({ clientToken: "Bearer token-123", sessionId: "session-123" }),
+			)
+			expect(mockShowInfo).toHaveBeenCalledWith("Successfully authenticated with Roo Code Cloud")
+		})
+
+		it("should handle Clerk API errors", async () => {
+			const storedState = "valid-state"
+			mockContext.globalState.get.mockReturnValue(storedState)
+
+			mockFetch.mockResolvedValue({
+				ok: false,
+				status: 400,
+				statusText: "Bad Request",
+			})
+
+			const loggedOutSpy = vi.fn()
+			authService.on("logged-out", loggedOutSpy)
+
+			await expect(authService.handleCallback("auth-code", storedState)).rejects.toThrow(
+				"Failed to handle Roo Code Cloud callback",
+			)
+			expect(loggedOutSpy).toHaveBeenCalled()
+		})
+	})
+
+	describe("logout", () => {
+		beforeEach(async () => {
+			await authService.initialize()
+		})
+
+		it("should clear credentials and call Clerk logout", async () => {
+			// Set up credentials first by simulating a login state
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+
+			// Manually set the credentials in the service
+			authService["credentials"] = credentials
+
+			// Mock successful logout response
+			mockFetch.mockResolvedValue({ ok: true })
+
+			const vscode = await import("vscode")
+			const mockShowInfo = vi.fn()
+			vi.mocked(vscode.window.showInformationMessage).mockImplementation(mockShowInfo)
+
+			await authService.logout()
+
+			expect(mockContext.secrets.delete).toHaveBeenCalledWith("clerk-auth-credentials")
+			expect(mockContext.globalState.update).toHaveBeenCalledWith("clerk-auth-state", undefined)
+			expect(mockFetch).toHaveBeenCalledWith(
+				"https://clerk.test.com/v1/client/sessions/test-session/remove",
+				expect.objectContaining({
+					method: "POST",
+					headers: expect.objectContaining({
+						Authorization: "Bearer test-token",
+					}),
+				}),
+			)
+			expect(mockShowInfo).toHaveBeenCalledWith("Logged out from Roo Code Cloud")
+		})
+
+		it("should handle logout without credentials", async () => {
+			const vscode = await import("vscode")
+			const mockShowInfo = vi.fn()
+			vi.mocked(vscode.window.showInformationMessage).mockImplementation(mockShowInfo)
+
+			await authService.logout()
+
+			expect(mockContext.secrets.delete).toHaveBeenCalled()
+			expect(mockFetch).not.toHaveBeenCalled()
+			expect(mockShowInfo).toHaveBeenCalledWith("Logged out from Roo Code Cloud")
+		})
+
+		it("should handle Clerk logout errors gracefully", async () => {
+			// Set up credentials first by simulating a login state
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+
+			// Manually set the credentials in the service
+			authService["credentials"] = credentials
+
+			// Mock failed logout response
+			mockFetch.mockRejectedValue(new Error("Network error"))
+
+			const vscode = await import("vscode")
+			const mockShowInfo = vi.fn()
+			vi.mocked(vscode.window.showInformationMessage).mockImplementation(mockShowInfo)
+
+			await authService.logout()
+
+			expect(mockLog).toHaveBeenCalledWith("[auth] Error calling clerkLogout:", expect.any(Error))
+			expect(mockShowInfo).toHaveBeenCalledWith("Logged out from Roo Code Cloud")
+		})
+	})
+
+	describe("state management", () => {
+		it("should return correct state", () => {
+			expect(authService.getState()).toBe("initializing")
+		})
+
+		it("should return correct authentication status", async () => {
+			await authService.initialize()
+			expect(authService.isAuthenticated()).toBe(false)
+
+			// Create a new service instance with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+
+			const authenticatedService = new AuthService(mockContext as unknown as vscode.ExtensionContext, mockLog)
+			await authenticatedService.initialize()
+
+			expect(authenticatedService.isAuthenticated()).toBe(true)
+			expect(authenticatedService.hasActiveSession()).toBe(false)
+		})
+
+		it("should return session token only for active sessions", () => {
+			expect(authService.getSessionToken()).toBeUndefined()
+
+			// Manually set state to active-session for testing
+			// This would normally happen through refreshSession
+			authService["state"] = "active-session"
+			authService["sessionToken"] = "test-jwt"
+
+			expect(authService.getSessionToken()).toBe("test-jwt")
+		})
+	})
+
+	describe("session refresh", () => {
+		beforeEach(async () => {
+			// Set up with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+			await authService.initialize()
+		})
+
+		it("should refresh session successfully", async () => {
+			// Mock successful token creation and user info fetch
+			mockFetch
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () => Promise.resolve({ jwt: "new-jwt-token" }),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: {
+								first_name: "John",
+								last_name: "Doe",
+								image_url: "https://example.com/avatar.jpg",
+								primary_email_address_id: "email-1",
+								email_addresses: [{ id: "email-1", email_address: "[email protected]" }],
+							},
+						}),
+				})
+
+			const activeSessionSpy = vi.fn()
+			const userInfoSpy = vi.fn()
+			authService.on("active-session", activeSessionSpy)
+			authService.on("user-info", userInfoSpy)
+
+			// Trigger refresh by calling the timer callback
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+			await timerCallback()
+
+			// Wait for async operations to complete
+			await new Promise((resolve) => setTimeout(resolve, 0))
+
+			expect(authService.getState()).toBe("active-session")
+			expect(authService.hasActiveSession()).toBe(true)
+			expect(authService.getSessionToken()).toBe("new-jwt-token")
+			expect(activeSessionSpy).toHaveBeenCalledWith({ previousState: "inactive-session" })
+			expect(userInfoSpy).toHaveBeenCalledWith({
+				userInfo: {
+					name: "John Doe",
+					email: "[email protected]",
+					picture: "https://example.com/avatar.jpg",
+				},
+			})
+		})
+
+		it("should handle invalid client token error", async () => {
+			// Mock 401 response (invalid token)
+			mockFetch.mockResolvedValue({
+				ok: false,
+				status: 401,
+				statusText: "Unauthorized",
+			})
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+
+			await expect(timerCallback()).rejects.toThrow()
+			expect(mockContext.secrets.delete).toHaveBeenCalledWith("clerk-auth-credentials")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Invalid/Expired client token: clearing credentials")
+		})
+
+		it("should handle network errors during refresh", async () => {
+			mockFetch.mockRejectedValue(new Error("Network error"))
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+
+			await expect(timerCallback()).rejects.toThrow("Network error")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Failed to refresh session", expect.any(Error))
+		})
+	})
+
+	describe("user info", () => {
+		it("should return null initially", () => {
+			expect(authService.getUserInfo()).toBeNull()
+		})
+
+		it("should parse user info correctly", async () => {
+			// Set up with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+			await authService.initialize()
+
+			// Clear previous mock calls
+			mockFetch.mockClear()
+
+			// Mock successful responses
+			mockFetch
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () => Promise.resolve({ jwt: "jwt-token" }),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: {
+								first_name: "Jane",
+								last_name: "Smith",
+								image_url: "https://example.com/jane.jpg",
+								primary_email_address_id: "email-2",
+								email_addresses: [
+									{ id: "email-1", email_address: "[email protected]" },
+									{ id: "email-2", email_address: "[email protected]" },
+								],
+							},
+						}),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: [
+								{
+									id: "org_member_id_1",
+									role: "member",
+									organization: {
+										id: "org_1",
+										name: "Org 1",
+									},
+								},
+							],
+						}),
+				})
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+			await timerCallback()
+
+			// Wait for async operations to complete
+			await new Promise((resolve) => setTimeout(resolve, 0))
+
+			const userInfo = authService.getUserInfo()
+			expect(userInfo).toEqual({
+				name: "Jane Smith",
+				email: "[email protected]",
+				picture: "https://example.com/jane.jpg",
+				organizationId: "org_1",
+				organizationName: "Org 1",
+				organizationRole: "member",
+			})
+		})
+
+		it("should handle missing user info fields", async () => {
+			// Set up with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+			await authService.initialize()
+
+			// Clear previous mock calls
+			mockFetch.mockClear()
+
+			// Mock responses with minimal data
+			mockFetch
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () => Promise.resolve({ jwt: "jwt-token" }),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: {
+								first_name: "John",
+								last_name: "Doe",
+								// Missing other fields
+							},
+						}),
+				})
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+			await timerCallback()
+
+			// Wait for async operations to complete
+			await new Promise((resolve) => setTimeout(resolve, 0))
+
+			const userInfo = authService.getUserInfo()
+			expect(userInfo).toEqual({
+				name: "John Doe",
+				email: undefined,
+				picture: undefined,
+			})
+		})
+	})
+
+	describe("event emissions", () => {
+		it("should emit logged-out event", async () => {
+			const loggedOutSpy = vi.fn()
+			authService.on("logged-out", loggedOutSpy)
+
+			await authService.initialize()
+
+			expect(loggedOutSpy).toHaveBeenCalledWith({ previousState: "initializing" })
+		})
+
+		it("should emit inactive-session event", async () => {
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+
+			const inactiveSessionSpy = vi.fn()
+			authService.on("inactive-session", inactiveSessionSpy)
+
+			await authService.initialize()
+
+			expect(inactiveSessionSpy).toHaveBeenCalledWith({ previousState: "initializing" })
+		})
+
+		it("should emit active-session event", async () => {
+			// Set up with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+			await authService.initialize()
+
+			// Clear previous mock calls
+			mockFetch.mockClear()
+
+			// Mock both the token creation and user info fetch
+			mockFetch
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () => Promise.resolve({ jwt: "jwt-token" }),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: {
+								first_name: "Test",
+								last_name: "User",
+							},
+						}),
+				})
+
+			const activeSessionSpy = vi.fn()
+			authService.on("active-session", activeSessionSpy)
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+			await timerCallback()
+
+			// Wait for async operations to complete
+			await new Promise((resolve) => setTimeout(resolve, 0))
+
+			expect(activeSessionSpy).toHaveBeenCalledWith({ previousState: "inactive-session" })
+		})
+
+		it("should emit user-info event", async () => {
+			// Set up with credentials
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+			await authService.initialize()
+
+			// Clear previous mock calls
+			mockFetch.mockClear()
+
+			mockFetch
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () => Promise.resolve({ jwt: "jwt-token" }),
+				})
+				.mockResolvedValueOnce({
+					ok: true,
+					json: () =>
+						Promise.resolve({
+							response: {
+								first_name: "Test",
+								last_name: "User",
+							},
+						}),
+				})
+
+			const userInfoSpy = vi.fn()
+			authService.on("user-info", userInfoSpy)
+
+			const timerCallback = vi.mocked(RefreshTimer).mock.calls[0][0].callback
+			await timerCallback()
+
+			// Wait for async operations to complete
+			await new Promise((resolve) => setTimeout(resolve, 0))
+
+			expect(userInfoSpy).toHaveBeenCalledWith({
+				userInfo: {
+					name: "Test User",
+					email: undefined,
+					picture: undefined,
+				},
+			})
+		})
+	})
+
+	describe("error handling", () => {
+		it("should handle credentials change errors", async () => {
+			mockContext.secrets.get.mockRejectedValue(new Error("Storage error"))
+
+			await authService.initialize()
+
+			expect(mockLog).toHaveBeenCalledWith("[auth] Error handling credentials change:", expect.any(Error))
+		})
+
+		it("should handle malformed JSON in credentials", async () => {
+			mockContext.secrets.get.mockResolvedValue("invalid-json{")
+
+			await authService.initialize()
+
+			expect(authService.getState()).toBe("logged-out")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Failed to parse stored credentials:", expect.any(Error))
+		})
+
+		it("should handle invalid credentials schema", async () => {
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify({ invalid: "data" }))
+
+			await authService.initialize()
+
+			expect(authService.getState()).toBe("logged-out")
+			expect(mockLog).toHaveBeenCalledWith("[auth] Invalid credentials format:", expect.any(Array))
+		})
+
+		it("should handle missing authorization header in sign-in response", async () => {
+			const storedState = "valid-state"
+			mockContext.globalState.get.mockReturnValue(storedState)
+
+			mockFetch.mockResolvedValue({
+				ok: true,
+				json: () =>
+					Promise.resolve({
+						response: { created_session_id: "session-123" },
+					}),
+				headers: {
+					get: () => null, // No authorization header
+				},
+			})
+
+			await expect(authService.handleCallback("auth-code", storedState)).rejects.toThrow(
+				"Failed to handle Roo Code Cloud callback",
+			)
+		})
+	})
+
+	describe("timer integration", () => {
+		it("should stop timer on logged-out transition", async () => {
+			await authService.initialize()
+
+			expect(mockTimer.stop).toHaveBeenCalled()
+		})
+
+		it("should start timer on inactive-session transition", async () => {
+			const credentials = { clientToken: "test-token", sessionId: "test-session" }
+			mockContext.secrets.get.mockResolvedValue(JSON.stringify(credentials))
+
+			await authService.initialize()
+
+			expect(mockTimer.start).toHaveBeenCalled()
+		})
+	})
+})

+ 5 - 8
packages/cloud/src/__tests__/CloudService.test.ts

@@ -79,7 +79,7 @@ describe("CloudService", () => {
 		} as unknown as vscode.ExtensionContext
 
 		mockAuthService = {
-			initialize: vi.fn(),
+			initialize: vi.fn().mockResolvedValue(undefined),
 			login: vi.fn(),
 			logout: vi.fn(),
 			isAuthenticated: vi.fn().mockReturnValue(false),
@@ -108,11 +108,8 @@ describe("CloudService", () => {
 			},
 		}
 
-		vi.mocked(AuthService.createInstance).mockResolvedValue(mockAuthService as unknown as AuthService)
-		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 })
+		vi.mocked(AuthService).mockImplementation(() => mockAuthService as unknown as AuthService)
+		vi.mocked(SettingsService).mockImplementation(() => mockSettingsService as unknown as SettingsService)
 
 		vi.mocked(TelemetryService.hasInstance).mockReturnValue(true)
 		Object.defineProperty(TelemetryService, "instance", {
@@ -135,8 +132,8 @@ describe("CloudService", () => {
 			const cloudService = await CloudService.createInstance(mockContext, callbacks)
 
 			expect(cloudService).toBeInstanceOf(CloudService)
-			expect(AuthService.createInstance).toHaveBeenCalledWith(mockContext, expect.any(Function))
-			expect(SettingsService.createInstance).toHaveBeenCalledWith(mockContext, expect.any(Function))
+			expect(AuthService).toHaveBeenCalledWith(mockContext, expect.any(Function))
+			expect(SettingsService).toHaveBeenCalledWith(mockContext, mockAuthService, expect.any(Function))
 		})
 
 		it("should throw error if instance already exists", async () => {