refactor(webview): consolidate view-local state persistence

Author: easonliang28

src/core/webview/ClineProvider.ts

@@ -566,17 +566,8 @@ 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 (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
-		}
+		await this._saveViewLocalStateFromMutation({ [key]: value } as Partial<RooCodeSettings> &
+			Partial<ExtensionState>)
 
 		this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`)
 	}
@@ -1855,11 +1846,9 @@ export class ClineProvider
 					this.updateGlobalState("currentApiConfigName", name),
 					this.providerSettingsManager.setModeConfig(mode, id),
 					this.contextProxy.setProviderSettings(providerSettings),
-					this.saveViewState("currentApiConfigName", name),
-					this.saveViewState("apiConfiguration", providerSettings),
 				])
 
-				this._updateViewLocalStateFromMutation({
+				await this._saveViewLocalStateFromMutation({
 					listApiConfigMeta,
 					currentApiConfigName: name,
 					apiConfiguration: providerSettings,
@@ -1959,11 +1948,9 @@ export class ClineProvider
 			this.contextProxy.setValue("listApiConfigMeta", listApiConfigMeta),
 			this.contextProxy.setValue("currentApiConfigName", name),
 			this.contextProxy.setProviderSettings(providerSettings),
-			this.saveViewState("currentApiConfigName", name),
-			this.saveViewState("apiConfiguration", providerSettings),
 		])
 
-		this._updateViewLocalStateFromMutation({
+		await this._saveViewLocalStateFromMutation({
 			listApiConfigMeta,
 			currentApiConfigName: name,
 			apiConfiguration: providerSettings,
@@ -3102,8 +3089,7 @@ export class ClineProvider
 
 	public async setValue<K extends keyof RooCodeSettings>(key: K, value: RooCodeSettings[K]) {
 		await this.contextProxy.setValue(key, value)
-		this._updateViewLocalStateFromMutation({ [key]: value })
-		await this._persistViewLocalStateFromMutation({ [key]: value })
+		await this._saveViewLocalStateFromMutation({ [key]: value })
 	}
 
 	public getValue<K extends keyof RooCodeSettings>(key: K) {
@@ -3116,8 +3102,14 @@ export class ClineProvider
 
 	public async setValues(values: RooCodeSettings) {
 		await this.contextProxy.setValues(values)
-		this._updateViewLocalStateFromMutation(values)
+		await this._saveViewLocalStateFromMutation(values)
+	}
+
+	private async _saveViewLocalStateFromMutation(
+		values: Partial<RooCodeSettings> & Partial<ExtensionState>,
+	): Promise<void> {
 		await this._persistViewLocalStateFromMutation(values)
+		this._updateViewLocalStateFromMutation(values)
 	}
 
 	/**

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

@@ -844,6 +844,24 @@ describe("ClineProvider - Parallel Mode Support", () => {
 
 			await provider.dispose()
 		})
+		it("should not update viewLocalState when durable view-state persistence fails", async () => {
+			const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
+			const providerAccess = provider as unknown as {
+				setViewStateId: (viewStateId: string) => Promise<void>
+				saveViewState: (key: keyof ExtensionState, value: unknown) => Promise<void>
+				viewLocalState: Partial<ExtensionState>
+			}
+			vi.spyOn(provider.contextProxy, "setValue").mockRejectedValueOnce(new Error("persist failed"))
+
+			await providerAccess.setViewStateId("stable-sidebar-view")
+
+			await expect(providerAccess.saveViewState("mode", "architect")).rejects.toThrow("persist failed")
+			expect(providerAccess.viewLocalState).not.toHaveProperty("mode")
+			expect(provider.contextProxy.getValue("viewStates")).toBeUndefined()
+
+			await provider.dispose()
+		})
+
 		it("should merge concurrent persisted updates from separate provider instances without lost viewStates", async () => {
 			const provider1 = new ClineProvider(
 				mockContext,
@@ -1175,6 +1193,7 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValueOnce([
 				{ id: "new-profile-id", name: "new-profile", apiProvider: "openrouter" },
 			] as any)
+			const saveViewStateSpy = vi.spyOn(provider as any, "saveViewState")
 			;(provider as any).viewLocalState = {
 				currentApiConfigName: "stale-profile",
 				apiConfiguration: { apiProvider: "anthropic" },
@@ -1183,6 +1202,7 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			await provider.activateProviderProfile({ name: "new-profile" })
 			const state = await provider.getState()
 
+			expect(saveViewStateSpy).not.toHaveBeenCalled()
 			expect(state.currentApiConfigName).toBe("new-profile")
 			expect(state.apiConfiguration).toMatchObject({
 				apiProvider: "openrouter",
@@ -1197,6 +1217,7 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([
 				{ id: "test-id", name: "saved-profile", apiProvider: "bedrock" },
 			] as any)
+			const saveViewStateSpy = vi.spyOn(provider as any, "saveViewState")
 			;(provider as any).viewLocalState = {
 				currentApiConfigName: "stale-profile",
 				apiConfiguration: { apiProvider: "anthropic" },
@@ -1208,6 +1229,7 @@ describe("ClineProvider - Parallel Mode Support", () => {
 			} as any)
 			const state = await provider.getState()
 
+			expect(saveViewStateSpy).not.toHaveBeenCalled()
 			expect(state.currentApiConfigName).toBe("saved-profile")
 			expect(state.apiConfiguration).toMatchObject({
 				apiProvider: "bedrock",

src/eslint-suppressions.json

@@ -1086,7 +1086,7 @@
 	},
 	"core/webview/__tests__/ClineProvider.parallelMode.spec.ts": {
 		"@typescript-eslint/no-explicit-any": {
-			"count": 141
+			"count": 143
 		}
 	},
 	"core/webview/__tests__/ClineProvider.spec.ts": {