fix(webview): address view-state review feedback

Author: easonliang28

apps/vscode-e2e/src/suite/view-state.test.ts

@@ -46,7 +46,9 @@ suite("Roo Code View State", function () {
 			}
 		}
 
-		globalThis.api.on(RooCodeEventName.TaskModeSwitched, (taskId, mode) => modeEvents.push({ taskId, mode }))
+		const modeHandler = (taskId: string, mode: string) => modeEvents.push({ taskId, mode })
+
+		globalThis.api.on(RooCodeEventName.TaskModeSwitched, modeHandler)
 		globalThis.api.on(RooCodeEventName.Message, completionHandler)
 
 		try {
@@ -110,6 +112,7 @@ suite("Roo Code View State", function () {
 				)
 			}
 		} finally {
+			globalThis.api.off(RooCodeEventName.TaskModeSwitched, modeHandler)
 			globalThis.api.off(RooCodeEventName.Message, completionHandler)
 		}
 	})

packages/types/src/api.ts

@@ -21,6 +21,7 @@ export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
 		text,
 		images,
 		newTab,
+		preserveOpenTabs,
 	}: {
 		configuration?: RooCodeSettings
 		text?: string

src/core/webview/ClineProvider.ts

@@ -955,15 +955,6 @@ export class ClineProvider
 		this.customModesManager?.dispose()
 		this.taskHistoryStore.dispose()
 		this.flushGlobalStateWriteThrough()
-		if (this.renderContext === "editor") {
-			try {
-				await this.clearPersistedViewState()
-			} catch (error) {
-				this.log(
-					`[dispose] Failed to clear persisted view state for ${this.viewStateId}: ${error instanceof Error ? error.message : String(error)}`,
-				)
-			}
-		}
 		this.log("Disposed all disposables")
 		ClineProvider.activeInstances.delete(this)
 

src/core/webview/__tests__/ClineProvider.parallelMode.spec.ts

@@ -1206,16 +1206,16 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			await provider.dispose()
 		})
 
-		it("should clean up persisted viewStates entry when a tab provider is disposed", async () => {
+		it("should preserve persisted viewStates entry when an editor provider is disposed during teardown", async () => {
 			const provider = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
 
-			await (provider as any).setViewStateId("tab-to-dispose")
+			await (provider as any).setViewStateId("tab-to-preserve")
 			await (provider as any).saveViewState("mode", "architect")
-			expect(provider.contextProxy.getValue("viewStates" as any)).toHaveProperty("tab-to-dispose")
+			expect(provider.contextProxy.getValue("viewStates" as any)).toHaveProperty("tab-to-preserve")
 
 			await provider.dispose()
 
-			expect(provider.contextProxy.getValue("viewStates" as any)).not.toHaveProperty("tab-to-dispose")
+			expect(provider.contextProxy.getValue("viewStates" as any)).toHaveProperty("tab-to-preserve")
 		})
 	})
 

src/core/webview/__tests__/ClineProvider.spec.ts

@@ -578,7 +578,7 @@ describe("ClineProvider", () => {
 	})
 
 	test("does not reload full model details when the LM Studio model is already loaded", async () => {
-		vi.mocked(hasLoadedFullDetails).mockReturnValue(true)
+		vi.mocked(hasLoadedFullDetails).mockReturnValueOnce(true)
 
 		await provider.performPreparationTasks({
 			apiConfiguration: {

src/core/webview/__tests__/webviewMessageHandler.routerModels.spec.ts

@@ -1,5 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from "vitest"
 import { webviewMessageHandler } from "../webviewMessageHandler"
+import type { WebviewMessage } from "@roo-code/types"
+
 import type { ClineProvider } from "../ClineProvider"
 
 // Mock vscode (minimal)
@@ -37,10 +39,16 @@ vi.mock("vscode", () => ({
 // Mock modelCache getModels/flushModels used by the handler
 const getModelsMock = vi.fn()
 const flushModelsMock = vi.fn()
+const kimiCodeGetAccessTokenMock = vi.fn()
 vi.mock("../../../api/providers/fetchers/modelCache", () => ({
 	getModels: (...args: any[]) => getModelsMock(...args),
 	flushModels: (...args: any[]) => flushModelsMock(...args),
 }))
+vi.mock("../../../integrations/kimi-code/oauth", () => ({
+	kimiCodeOAuthManager: {
+		getAccessToken: (...args: unknown[]) => kimiCodeGetAccessTokenMock(...args),
+	},
+}))
 
 describe("webviewMessageHandler - requestRouterModels provider filter", () => {
 	let mockProvider: ClineProvider & {
@@ -296,6 +304,31 @@ describe("webviewMessageHandler - requestRouterModels provider filter", () => {
 		})
 	})
 
+	it("continues posting routerModels when Kimi Code OAuth lookup fails", async () => {
+		mockProvider.getState.mockResolvedValue({
+			apiConfiguration: {
+				kimiCodeAuthMethod: "oauth",
+			},
+		})
+		kimiCodeGetAccessTokenMock.mockRejectedValueOnce(new Error("refresh failed"))
+
+		await webviewMessageHandler(mockProvider, {
+			type: "requestRouterModels",
+			values: { provider: "kimi-code" },
+		} satisfies WebviewMessage)
+
+		expect(kimiCodeGetAccessTokenMock).toHaveBeenCalledOnce()
+		expect(mockProvider.log).toHaveBeenCalledWith(
+			"[requestRouterModels] kimi-code credential lookup failed: refresh failed",
+		)
+		expect(getModelsMock).not.toHaveBeenCalledWith(expect.objectContaining({ provider: "kimi-code" }))
+		expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
+			type: "routerModels",
+			routerModels: {},
+			values: { provider: "kimi-code" },
+		})
+	})
+
 	it("fetches Moonshot models when stored Moonshot credentials exist", async () => {
 		mockProvider.getState.mockResolvedValue({
 			apiConfiguration: {

src/core/webview/webviewMessageHandler.ts

@@ -558,7 +558,7 @@ export const webviewMessageHandler = async (
 	}
 
 	switch (message.type) {
-		case "webviewDidLaunch":
+		case "webviewDidLaunch": {
 			await provider.setViewStateId(message.viewStateId)
 
 			// Load custom modes first
@@ -645,6 +645,7 @@ export const webviewMessageHandler = async (
 
 			provider.isViewLaunched = true
 			break
+		}
 		case "newTask":
 			// Initializing new instance of Cline will make sure that any
 			// agentically running promises in old instance don't affect our new
@@ -1223,18 +1224,24 @@ export const webviewMessageHandler = async (
 			})
 
 			if (!providerFilter || providerFilter === "kimi-code") {
-				const { kimiCodeOAuthManager } = await import("../../integrations/kimi-code/oauth")
-				const kimiCodeAuthMethod =
-					message?.values?.kimiCodeAuthMethod ?? apiConfiguration.kimiCodeAuthMethod ?? "oauth"
-				const kimiCodeApiKey =
-					kimiCodeAuthMethod === "api-key"
-						? (message?.values?.kimiCodeApiKey ?? apiConfiguration.kimiCodeApiKey)
-						: await kimiCodeOAuthManager.getAccessToken()
-				if (kimiCodeApiKey) {
-					candidates.push({
-						key: "kimi-code",
-						options: { provider: "kimi-code", apiKey: kimiCodeApiKey },
-					})
+				try {
+					const { kimiCodeOAuthManager } = await import("../../integrations/kimi-code/oauth")
+					const kimiCodeAuthMethod =
+						message?.values?.kimiCodeAuthMethod ?? apiConfiguration.kimiCodeAuthMethod ?? "oauth"
+					const kimiCodeApiKey =
+						kimiCodeAuthMethod === "api-key"
+							? (message?.values?.kimiCodeApiKey ?? apiConfiguration.kimiCodeApiKey)
+							: await kimiCodeOAuthManager.getAccessToken()
+					if (kimiCodeApiKey) {
+						candidates.push({
+							key: "kimi-code",
+							options: { provider: "kimi-code", apiKey: kimiCodeApiKey },
+						})
+					}
+				} catch (error) {
+					provider.log(
+						`[requestRouterModels] kimi-code credential lookup failed: ${error instanceof Error ? error.message : String(error)}`,
+					)
 				}
 			}
 
@@ -1386,11 +1393,12 @@ export const webviewMessageHandler = async (
 			}
 
 			break
-		case "requestVsCodeLmModels":
+		case "requestVsCodeLmModels": {
 			const vsCodeLmModels = await getVsCodeLmModels()
 			// TODO: Cache like we do for OpenRouter, etc?
 			await provider.postMessageToWebview({ type: "vsCodeLmModels", vsCodeLmModels })
 			break
+		}
 		case "openImage":
 			await openImage(message.text!, { values: message.values })
 			break

src/extension/__tests__/api-task-control.spec.ts

@@ -170,7 +170,7 @@ describe("API task controls", () => {
 			expect(task.approveAsk).toHaveBeenCalledOnce()
 		})
 
-		it("removes completed and aborted tasks from the registry", async () => {
+		it("removes completed, aborted, and unfocused tasks from the registry", async () => {
 			const completedTask = createTask("completed-task")
 			sidebarProvider.emit(RooCodeEventName.TaskCreated, completedTask)
 			completedTask.emit(RooCodeEventName.TaskCompleted, completedTask.taskId, {}, {})
@@ -182,6 +182,12 @@ describe("API task controls", () => {
 			abortedTask.emit(RooCodeEventName.TaskAborted)
 
 			await expect(api.approveTaskAsk(abortedTask.taskId)).resolves.toBe(false)
+
+			const unfocusedTask = createTask("unfocused-task")
+			sidebarProvider.emit(RooCodeEventName.TaskCreated, unfocusedTask)
+			unfocusedTask.emit(RooCodeEventName.TaskUnfocused)
+
+			await expect(api.approveTaskAsk(unfocusedTask.taskId)).resolves.toBe(false)
 		})
 	})
 
@@ -218,8 +224,9 @@ describe("API task controls", () => {
 			expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Use architect")
 		})
 
-		it("responds without switching modes when the requested mode is invalid", async () => {
+		it("responds without switching modes and logs when the requested mode is invalid", async () => {
 			const task = createTask("task-invalid-mode")
+			api = new API(outputChannel, asClineProvider(sidebarProvider), undefined, true)
 			sidebarProvider.emit(RooCodeEventName.TaskCreated, task)
 
 			await expect(
@@ -229,6 +236,9 @@ describe("API task controls", () => {
 			expect(sidebarProvider.getState).toHaveBeenCalledOnce()
 			expect(sidebarProvider.handleModeSwitch).not.toHaveBeenCalled()
 			expect(task.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Use invalid")
+			expect(outputChannel.appendLine).toHaveBeenCalledWith(
+				'[API#selectTaskFollowupSuggestion] ignoring unknown mode "not-a-mode" for task task-invalid-mode',
+			)
 		})
 
 		it("treats custom modes from the task provider state as valid", async () => {

src/extension/api.ts

@@ -358,6 +358,8 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
 
 			if (isValidMode) {
 				await entry.provider.handleModeSwitch(mode)
+			} else {
+				this.log(`[API#selectTaskFollowupSuggestion] ignoring unknown mode "${mode}" for task ${taskId}`)
 			}
 		}
 
@@ -416,6 +418,7 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
 
 			task.on(RooCodeEventName.TaskUnfocused, () => {
 				this.emit(RooCodeEventName.TaskUnfocused, task.taskId)
+				this.tasksById.delete(task.taskId)
 			})
 
 			task.on(RooCodeEventName.TaskActive, () => {