fix(webview): preserve isolated view state writes

Author: easonliang28

src/core/webview/ClineProvider.ts

@@ -459,9 +459,10 @@ export class ClineProvider
 	}
 
 	private async savePersistedViewState(values: Partial<PersistedViewState>): Promise<void> {
+		const viewStateId = this.viewStateId
 		const write = ClineProvider.persistedViewStateWriteQueue.then(async () => {
 			const states = this.getPersistedViewStates({ fresh: true })
-			const current = states[this.viewStateId] ?? {}
+			const current = states[viewStateId] ?? {}
 			const next: PersistedViewState = { ...current }
 
 			if ("mode" in values) {
@@ -481,10 +482,10 @@ export class ClineProvider
 			}
 
 			if (!next.mode && !next.currentApiConfigName) {
-				delete states[this.viewStateId]
+				delete states[viewStateId]
 			} else {
 				next.updatedAt = values.updatedAt ?? Date.now()
-				states[this.viewStateId] = next
+				states[viewStateId] = next
 			}
 
 			await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states))
@@ -565,18 +566,18 @@ export class ClineProvider
 	 * Save a single view-local state value. Only non-secret selections are persisted durably.
 	 */
 	private async saveViewState(key: keyof ExtensionState, value: any): Promise<void> {
-		if (value === undefined || value === null) {
-			delete this.viewLocalState[key]
-		} else {
-			this.viewLocalState[key] = value
-		}
-
 		if (key === "mode") {
 			await this.savePersistedViewState({ mode: value })
 		} else if (key === "currentApiConfigName") {
 			await this.savePersistedViewState({ currentApiConfigName: value })
 		}
 
+		if (value === undefined || value === null) {
+			delete this.viewLocalState[key]
+		} else {
+			this.viewLocalState[key] = value
+		}
+
 		this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`)
 	}
 
@@ -906,6 +907,15 @@ 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)
 
@@ -3150,10 +3160,13 @@ export class ClineProvider
 				return acc
 			}, {} as ProviderSettings)
 
-			this.viewLocalState.apiConfiguration = {
-				...(this.viewLocalState.apiConfiguration ?? {}),
-				...providerSettingsUpdate,
-			}
+			this.viewLocalState.apiConfiguration =
+				"apiProvider" in providerSettingsUpdate
+					? providerSettingsUpdate
+					: {
+							...(this.viewLocalState.apiConfiguration ?? {}),
+							...providerSettingsUpdate,
+						}
 		}
 	}
 

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

@@ -198,7 +198,7 @@ vi.mock("vscode", () => ({
 		showErrorMessage: vi.fn(),
 		activeTextEditor: undefined,
 		onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })),
-		createTextEditorDecorationType: vi.fn().mockReturnValue({}),
+		createTextEditorDecorationType: vi.fn().mockReturnValue({ dispose: vi.fn() }),
 		tabGroups: {
 			onDidChangeTabs: vi.fn().mockReturnValue({ dispose: vi.fn() }),
 		},
@@ -691,13 +691,14 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			)
 			const provider2 = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
 
+			await (provider2 as any).saveViewState("mode", "debugger")
 			await (provider1 as any).saveViewState("mode", "architect")
 
 			const state1 = await provider1.getState()
 			const state2 = await provider2.getState()
 
 			expect(state1.mode).toBe("architect")
-			expect(state2.mode).toBe("code")
+			expect(state2.mode).toBe("debugger")
 
 			await provider1.dispose()
 			await provider2.dispose()
@@ -1064,6 +1065,7 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			expect(state.apiConfiguration.apiProvider).toBe("bedrock")
 			expect(state.apiConfiguration.awsBedrockEndpoint).toBe("http://127.0.0.1:4567")
 			expect((provider as any).viewLocalState.apiConfiguration.apiProvider).toBe("bedrock")
+			expect((provider as any).viewLocalState.apiConfiguration).not.toHaveProperty("openRouterModelId")
 
 			await provider.dispose()
 		})
@@ -1093,6 +1095,72 @@ describe("ClineProvider - Parallel Mode Support", () => {
 
 			await provider.dispose()
 		})
+
+		it("should sanitize raw viewStateId before using it as persisted viewStates key", async () => {
+			const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
+
+			await (provider as any).setViewStateId("tab panel/with.dots and spaces")
+			await provider.setValue("mode" as any, "architect" as any)
+
+			expect(provider.contextProxy.getValue("viewStates" as any)).toMatchObject({
+				tab_panel_with_dots_and_spaces: { mode: "architect" },
+			})
+			expect(provider.contextProxy.getValue("viewStates" as any)).not.toHaveProperty(
+				"tab panel/with.dots and spaces",
+			)
+
+			await provider.dispose()
+		})
+
+		it("should persist queued writes under the viewStateId active when the change was made", async () => {
+			let releaseFirstWrite!: () => void
+			const firstWriteStarted = new Promise<void>((resolve) => {
+				mockContext.globalState.update = vi
+					.fn()
+					.mockImplementationOnce((key: string, value: any) => {
+						mockContext.globalState.get = vi
+							.fn()
+							.mockImplementation((lookupKey: string) => (lookupKey === key ? value : undefined))
+						resolve()
+						return new Promise<void>((writeResolve) => {
+							releaseFirstWrite = writeResolve
+						})
+					})
+					.mockImplementation((key: string, value: any) => {
+						mockContext.globalState.get = vi
+							.fn()
+							.mockImplementation((lookupKey: string) => (lookupKey === key ? value : undefined))
+						return Promise.resolve()
+					})
+			})
+			const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
+
+			await (provider as any).setViewStateId("view-a")
+			const firstSave = (provider as any).saveViewState("mode", "architect")
+			await firstWriteStarted
+			await (provider as any).setViewStateId("view-b")
+			releaseFirstWrite()
+			await firstSave
+
+			expect(provider.contextProxy.getValue("viewStates" as any)).toMatchObject({
+				"view-a": { mode: "architect" },
+			})
+			expect(provider.contextProxy.getValue("viewStates" as any)).not.toHaveProperty("view-b")
+
+			await provider.dispose()
+		})
+
+		it("should clean up persisted viewStates entry when a tab provider is disposed", async () => {
+			const provider = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
+
+			await (provider as any).setViewStateId("tab-to-dispose")
+			await (provider as any).saveViewState("mode", "architect")
+			expect(provider.contextProxy.getValue("viewStates" as any)).toHaveProperty("tab-to-dispose")
+
+			await provider.dispose()
+
+			expect(provider.contextProxy.getValue("viewStates" as any)).not.toHaveProperty("tab-to-dispose")
+		})
 	})
 
 	describe("profile mutations", () => {

webview-ui/src/utils/__tests__/vscode.spec.ts

@@ -61,9 +61,10 @@ describe("VSCodeAPIWrapper", () => {
 	})
 
 	it("falls back to in-memory state when browser storage access is restricted", () => {
+		const randomUUID = vi.fn().mockReturnValueOnce("memory-view").mockReturnValueOnce("new-memory-view")
 		Object.defineProperty(globalThis, "crypto", {
 			configurable: true,
-			value: { randomUUID: vi.fn(() => "memory-view") },
+			value: { randomUUID },
 		})
 		const storage = {
 			getItem: vi.fn(() => {
@@ -81,6 +82,7 @@ describe("VSCodeAPIWrapper", () => {
 
 		expect(wrapper.getViewStateId()).toBe("memory-view")
 		expect(wrapper.getViewStateId()).toBe("memory-view")
+		expect(randomUUID).toHaveBeenCalledTimes(1)
 		expect(storage.getItem).toHaveBeenCalled()
 		expect(storage.setItem).toHaveBeenCalled()
 	})