Skip to content

Commit 0137651

Browse files
committed
fix(webview): persist per-view selections through registered global state
1 parent 085f614 commit 0137651

4 files changed

Lines changed: 175 additions & 49 deletions

File tree

packages/types/src/__tests__/index.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
import { GLOBAL_STATE_KEYS } from "../index.js"
44

55
describe("GLOBAL_STATE_KEYS", () => {
6+
it("should contain registered durable per-view state", () => {
7+
expect(GLOBAL_STATE_KEYS).toContain("viewStates")
8+
})
9+
610
it("should contain provider settings keys", () => {
711
expect(GLOBAL_STATE_KEYS).toContain("autoApprovalEnabled")
812
})
@@ -13,6 +17,7 @@ describe("GLOBAL_STATE_KEYS", () => {
1317

1418
it("should not contain secret state keys", () => {
1519
expect(GLOBAL_STATE_KEYS).not.toContain("openRouterApiKey")
20+
expect(GLOBAL_STATE_KEYS).not.toContain("apiKey")
1621
})
1722

1823
it("should contain OpenAI Compatible base URL setting", () => {

packages/types/src/global-settings.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,15 @@ export const MAX_CHECKPOINT_TIMEOUT_SECONDS = 60
9797
*/
9898
export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15
9999

100+
/**
101+
* Persisted non-secret selections for a stable webview instance.
102+
*/
103+
export const viewStateSchema = z.object({
104+
mode: z.string().optional(),
105+
currentApiConfigName: z.string().optional(),
106+
updatedAt: z.number().optional(),
107+
})
108+
100109
/**
101110
* GlobalSettings
102111
*/
@@ -105,6 +114,7 @@ export const globalSettingsSchema = z.object({
105114
currentApiConfigName: z.string().optional(),
106115
listApiConfigMeta: z.array(providerSettingsEntrySchema).optional(),
107116
pinnedApiConfigs: z.record(z.string(), z.boolean()).optional(),
117+
viewStates: z.record(z.string(), viewStateSchema).optional(),
108118

109119
lastShownAnnouncementId: z.string().optional(),
110120
customInstructions: z.string().optional(),

src/core/webview/ClineProvider.ts

Lines changed: 79 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty"
117117
import { validateAndFixToolResultIds } from "../task/validateToolResultIds"
118118
import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore"
119119

120+
type PersistedViewState = NonNullable<GlobalState["viewStates"]>[string]
121+
120122
/**
121123
* https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts
122124
* https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts
@@ -163,6 +165,7 @@ export class ClineProvider
163165
public static readonly tabPanelId = `${Package.name}.TabPanelProvider`
164166
private static activeInstances: Set<ClineProvider> = new Set()
165167
private static nextViewId = 0
168+
private static readonly MAX_PERSISTED_VIEW_STATES = 50
166169
private disposables: vscode.Disposable[] = []
167170
private webviewDisposables: vscode.Disposable[] = []
168171
private view?: vscode.WebviewView | vscode.WebviewPanel
@@ -425,13 +428,59 @@ export class ClineProvider
425428
}
426429
}
427430

428-
/**
429-
* Derive a view-specific ContextProxy key for persisting view-local state.
430-
* Uses a stable per-view id so each restored tab reads and writes its own values
431-
* independent of provider construction order.
432-
*/
433-
private viewStateKeyFor(key: "mode" | "currentApiConfigName" | "apiConfiguration"): string {
434-
return `__view_state_${this.viewStateId}_${key}`
431+
private getPersistedViewStates(): Record<string, PersistedViewState> {
432+
const viewStates = this.contextProxy.getValue("viewStates")
433+
434+
if (!viewStates || typeof viewStates !== "object" || Array.isArray(viewStates)) {
435+
return {}
436+
}
437+
438+
return viewStates
439+
}
440+
441+
private async savePersistedViewState(values: Partial<PersistedViewState>): Promise<void> {
442+
const states = this.getPersistedViewStates()
443+
const current = states[this.viewStateId] ?? {}
444+
const next: PersistedViewState = { ...current }
445+
446+
if ("mode" in values) {
447+
if (values.mode === undefined || values.mode === null) {
448+
delete next.mode
449+
} else {
450+
next.mode = values.mode
451+
}
452+
}
453+
454+
if ("currentApiConfigName" in values) {
455+
if (values.currentApiConfigName === undefined || values.currentApiConfigName === null) {
456+
delete next.currentApiConfigName
457+
} else {
458+
next.currentApiConfigName = values.currentApiConfigName
459+
}
460+
}
461+
462+
if (!next.mode && !next.currentApiConfigName) {
463+
delete states[this.viewStateId]
464+
} else {
465+
next.updatedAt = values.updatedAt ?? Date.now()
466+
states[this.viewStateId] = next
467+
}
468+
469+
await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states))
470+
}
471+
472+
private async clearPersistedViewState(viewStateId = this.viewStateId): Promise<void> {
473+
const states = this.getPersistedViewStates()
474+
delete states[viewStateId]
475+
await this.contextProxy.setValue("viewStates", states)
476+
}
477+
478+
private prunePersistedViewStates(states: Record<string, PersistedViewState>): Record<string, PersistedViewState> {
479+
return Object.fromEntries(
480+
Object.entries(states)
481+
.sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0))
482+
.slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES),
483+
)
435484
}
436485

437486
public async setViewStateId(viewStateId: string | undefined): Promise<void> {
@@ -446,18 +495,30 @@ export class ClineProvider
446495
}
447496

448497
/**
449-
* Loads persisted values from stable per-view keys into the view-local state buffer.
450-
* Missing keys are intentionally left unset so getState() falls back to shared ContextProxy values.
498+
* Loads non-secret persisted selections from the registered viewStates map.
499+
* Missing entries are intentionally left unset so getState() falls back to shared ContextProxy values.
451500
*/
452501
private async loadViewState(): Promise<void> {
453502
try {
503+
const persisted = this.getPersistedViewStates()[this.viewStateId]
454504
const loadedState: Partial<ExtensionState> = {}
455505

456-
for (const key of ["mode", "currentApiConfigName", "apiConfiguration"] as const) {
457-
const value = this.contextProxy.getValue(this.viewStateKeyFor(key) as any)
506+
if (persisted?.mode) {
507+
loadedState.mode = persisted.mode as Mode
508+
}
509+
510+
if (persisted?.currentApiConfigName) {
511+
loadedState.currentApiConfigName = persisted.currentApiConfigName
458512

459-
if (value !== undefined && value !== null) {
460-
loadedState[key] = value as any
513+
try {
514+
const { name: _name, ...apiConfiguration } = await this.providerSettingsManager.getProfile({
515+
name: persisted.currentApiConfigName,
516+
})
517+
loadedState.apiConfiguration = apiConfiguration as ProviderSettings
518+
} catch (error) {
519+
this.log(
520+
`[loadViewState] Unable to resolve API profile '${persisted.currentApiConfigName}' for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`,
521+
)
461522
}
462523
}
463524

@@ -471,22 +532,19 @@ export class ClineProvider
471532
}
472533

473534
/**
474-
* Save a single view-local state value and sync to global state using a view-specific key.
475-
* This allows each Provider instance to have its own mode/apiConfig for parallel mode support.
535+
* Save a single view-local state value. Only non-secret selections are persisted durably.
476536
*/
477537
private async saveViewState(key: keyof ExtensionState, value: any): Promise<void> {
478-
// Update local cache first. Undefined/null clears should not leave a local override behind.
479538
if (value === undefined || value === null) {
480539
delete this.viewLocalState[key]
481540
} else {
482541
this.viewLocalState[key] = value
483542
}
484543

485-
// Persist to view-specific ContextProxy key for mode/currentApiConfigName/apiConfiguration,
486-
// so recreated views restore their own values instead of the last writer's shared state.
487-
if (key === "mode" || key === "currentApiConfigName" || key === "apiConfiguration") {
488-
const viewKey = this.viewStateKeyFor(key as "mode" | "currentApiConfigName" | "apiConfiguration")
489-
await this.contextProxy.setValue(viewKey as any, value)
544+
if (key === "mode") {
545+
await this.savePersistedViewState({ mode: value })
546+
} else if (key === "currentApiConfigName") {
547+
await this.savePersistedViewState({ currentApiConfigName: value })
490548
}
491549

492550
this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`)

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

Lines changed: 81 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -736,45 +736,60 @@ describe("ClineProvider - Parallel Mode Support", () => {
736736
})
737737

738738
describe("saveViewState", () => {
739-
it("should update viewLocalState when saveViewState is called", async () => {
739+
it("should update viewLocalState and persist mode through registered viewStates", async () => {
740740
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
741741

742742
const contextProxySpy = vi.spyOn(provider.contextProxy, "setValue")
743743
await (provider as any).setViewStateId("stable-sidebar-view")
744744

745745
await (provider as any).saveViewState("mode", "architect")
746746

747-
// Verify viewLocalState was updated
748747
expect((provider as any).viewLocalState.mode).toBe("architect")
749-
750-
// saveViewState uses the stable per-view id, not the construction-order viewId suffix.
751-
expect(contextProxySpy).toHaveBeenCalledWith("__view_state_stable-sidebar-view_mode", "architect")
752-
expect(contextProxySpy).not.toHaveBeenCalledWith(`__view_state_${provider.viewId}_mode`, expect.anything())
748+
expect(provider.contextProxy.getValue("viewStates" as any)).toMatchObject({
749+
"stable-sidebar-view": { mode: "architect" },
750+
})
751+
expect(contextProxySpy).toHaveBeenCalledWith(
752+
"viewStates",
753+
expect.objectContaining({
754+
"stable-sidebar-view": expect.objectContaining({
755+
mode: "architect",
756+
updatedAt: expect.any(Number),
757+
}),
758+
}),
759+
)
760+
expect(contextProxySpy).not.toHaveBeenCalledWith("__view_state_stable-sidebar-view_mode", expect.anything())
753761

754762
await provider.dispose()
755763
})
756764

757-
it("should update viewLocalState for currentApiConfigName", async () => {
765+
it("should update viewLocalState and persist currentApiConfigName through registered viewStates", async () => {
758766
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
759767

768+
await (provider as any).setViewStateId("stable-sidebar-view")
760769
await (provider as any).saveViewState("currentApiConfigName", "my-profile")
761770

762771
expect((provider as any).viewLocalState.currentApiConfigName).toBe("my-profile")
772+
expect(provider.contextProxy.getValue("viewStates" as any)).toMatchObject({
773+
"stable-sidebar-view": { currentApiConfigName: "my-profile" },
774+
})
763775

764776
await provider.dispose()
765777
})
766778

767-
it("should update viewLocalState for apiConfiguration", async () => {
779+
it("should update viewLocalState for apiConfiguration without persisting provider settings or secrets", async () => {
768780
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
769781

770782
const testApiConfig = {
771783
apiProvider: "openrouter" as const,
772784
openRouterModelId: "claude-3.5-sonnet",
785+
openRouterApiKey: "secret-key",
773786
}
774787

788+
await (provider as any).setViewStateId("stable-sidebar-view")
775789
await (provider as any).saveViewState("apiConfiguration", testApiConfig)
776790

777791
expect((provider as any).viewLocalState.apiConfiguration).toEqual(testApiConfig)
792+
expect(provider.contextProxy.getValue("viewStates" as any)).toBeUndefined()
778793

779794
await provider.dispose()
780795
})
@@ -823,15 +838,13 @@ describe("ClineProvider - Parallel Mode Support", () => {
823838
await provider.dispose()
824839
})
825840

826-
it("should update viewLocalState when stable per-view values are loaded manually", async () => {
841+
it("should restore mode and currentApiConfigName from hydrated viewStates after extension reload", async () => {
827842
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
828843
const stableViewId = "stable-sidebar-view"
829844

830-
await provider.contextProxy.setValue(`__view_state_${stableViewId}_mode` as any, "architect")
831-
await provider.contextProxy.setValue(
832-
`__view_state_${stableViewId}_currentApiConfigName` as any,
833-
"new-profile",
834-
)
845+
await provider.contextProxy.setValue("viewStates" as any, {
846+
[stableViewId]: { mode: "architect", currentApiConfigName: "new-profile", updatedAt: 123 },
847+
})
835848

836849
await (provider as any).setViewStateId(stableViewId)
837850

@@ -842,33 +855,52 @@ describe("ClineProvider - Parallel Mode Support", () => {
842855
await provider.dispose()
843856
})
844857

845-
it("should restore mode, current API config name, and API configuration from stable per-view state", async () => {
858+
it("should resolve API configuration from the persisted profile selection", async () => {
846859
const provider = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
847860
const stableViewId = "stable-editor-tab-a"
848-
const persistedApiConfiguration = {
849-
apiProvider: "openrouter" as const,
861+
const getProfileSpy = vi.spyOn(provider.providerSettingsManager, "getProfile").mockResolvedValue({
862+
name: "profile-a",
863+
id: "profile-a-id",
864+
apiProvider: "openrouter",
850865
openRouterModelId: "openrouter/anthropic/claude-sonnet-4",
851-
}
866+
} as any)
852867

853-
await provider.contextProxy.setValue(`__view_state_${stableViewId}_mode` as any, "architect")
854-
await provider.contextProxy.setValue(
855-
`__view_state_${stableViewId}_currentApiConfigName` as any,
856-
"profile-a",
857-
)
858-
await provider.contextProxy.setValue(
859-
`__view_state_${stableViewId}_apiConfiguration` as any,
860-
persistedApiConfiguration,
861-
)
868+
await provider.contextProxy.setValue("viewStates" as any, {
869+
[stableViewId]: { mode: "architect", currentApiConfigName: "profile-a", updatedAt: 123 },
870+
})
862871
await provider.contextProxy.setValue("mode" as any, "debugger")
863872
await provider.contextProxy.setValue("currentApiConfigName" as any, "profile-b")
864873
await provider.contextProxy.setValue("apiConfiguration" as any, { apiProvider: "anthropic" })
865874

866875
await (provider as any).setViewStateId(stableViewId)
867876
const state = await provider.getState()
868877

878+
expect(getProfileSpy).toHaveBeenCalledWith({ name: "profile-a" })
869879
expect(state.mode).toBe("architect")
870880
expect(state.currentApiConfigName).toBe("profile-a")
871-
expect(state.apiConfiguration).toMatchObject(persistedApiConfiguration)
881+
expect(state.apiConfiguration).toMatchObject({
882+
apiProvider: "openrouter",
883+
openRouterModelId: "openrouter/anthropic/claude-sonnet-4",
884+
})
885+
886+
await provider.dispose()
887+
})
888+
889+
it("should not throw when a persisted profile selection cannot be resolved", async () => {
890+
const provider = new ClineProvider(mockContext, mockOutputChannel, "editor", new ContextProxy(mockContext))
891+
const stableViewId = "stable-editor-tab-a"
892+
vi.spyOn(provider.providerSettingsManager, "getProfile").mockRejectedValue(new Error("missing profile"))
893+
894+
await provider.contextProxy.setValue("viewStates" as any, {
895+
[stableViewId]: { mode: "architect", currentApiConfigName: "deleted-profile", updatedAt: 123 },
896+
})
897+
898+
await expect((provider as any).setViewStateId(stableViewId)).resolves.toBeUndefined()
899+
const state = await provider.getState()
900+
901+
expect(state.mode).toBe("architect")
902+
expect(state.currentApiConfigName).toBe("deleted-profile")
903+
expect(state.apiConfiguration.apiProvider).toBe("anthropic")
872904

873905
await provider.dispose()
874906
})
@@ -891,6 +923,27 @@ describe("ClineProvider - Parallel Mode Support", () => {
891923
})
892924
})
893925

926+
describe("persisted view state pruning", () => {
927+
it("should keep the newest 50 persisted view states", async () => {
928+
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))
929+
const states = Object.fromEntries(
930+
Array.from({ length: 55 }, (_, index) => [
931+
`view-${index}`,
932+
{ mode: `mode-${index}`, updatedAt: index },
933+
]),
934+
)
935+
936+
const pruned = (provider as any).prunePersistedViewStates(states)
937+
938+
expect(Object.keys(pruned)).toHaveLength(50)
939+
expect(pruned["view-54"]).toBeDefined()
940+
expect(pruned["view-5"]).toBeDefined()
941+
expect(pruned["view-4"]).toBeUndefined()
942+
943+
await provider.dispose()
944+
})
945+
})
946+
894947
describe("getState merging", () => {
895948
it("should merge viewLocalState on top of global state", async () => {
896949
const provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext))

0 commit comments

Comments
 (0)