diff --git a/packages/types/src/__tests__/global-settings.test.ts b/packages/types/src/__tests__/global-settings.test.ts index f2b2e7e8e5..b6b01cccf0 100644 --- a/packages/types/src/__tests__/global-settings.test.ts +++ b/packages/types/src/__tests__/global-settings.test.ts @@ -39,6 +39,49 @@ describe("alwaysDenyUnapprovedCommands global setting", () => { }) }) +describe("viewStates global setting", () => { + it("parses a valid per-view record", () => { + const parsed = globalSettingsSchema.parse({ + viewStates: { + "stable-view-1": { mode: "code", currentApiConfigName: "profile-a", updatedAt: 1730000000000 }, + "stable-view-2": {}, + }, + }) + expect(parsed.viewStates?.["stable-view-1"]).toEqual({ + mode: "code", + currentApiConfigName: "profile-a", + updatedAt: 1730000000000, + }) + expect(parsed.viewStates?.["stable-view-2"]).toEqual({}) + expect(GLOBAL_SETTINGS_KEYS).toContain("viewStates") + }) + + it("parses an entry that carries the host-minted claim token", () => { + // The claim binds the entry to the webview that wrote it; a legacy entry without one + // still parses, because refusing it is setViewStateId's job, not the schema's. + const parsed = globalSettingsSchema.parse({ + viewStates: { + "stable-view-1": { mode: "code", updatedAt: 1730000000000, claimToken: "7f9c-host-minted" }, + "stable-view-2": { mode: "code" }, + }, + }) + expect(parsed.viewStates?.["stable-view-1"]?.claimToken).toBe("7f9c-host-minted") + expect(parsed.viewStates?.["stable-view-2"]).toEqual({ mode: "code" }) + }) + + it("rejects invalid field values inside a view entry", () => { + expect(() => globalSettingsSchema.parse({ viewStates: { v: { mode: 42 } } })).toThrow() + expect(() => globalSettingsSchema.parse({ viewStates: { v: { currentApiConfigName: 42 } } })).toThrow() + expect(() => globalSettingsSchema.parse({ viewStates: { v: { updatedAt: "soon" } } })).toThrow() + expect(() => globalSettingsSchema.parse({ viewStates: { v: { claimToken: 42 } } })).toThrow() + }) + + it("rejects non-object viewStates values", () => { + expect(() => globalSettingsSchema.parse({ viewStates: "not-a-record" })).toThrow() + expect(() => globalSettingsSchema.parse({ viewStates: { v: "not-an-entry" } })).toThrow() + }) +}) + describe("collapseCommandOutputByDefault global setting", () => { it.each([true, false, undefined])("round-trips %s through the export/import schema", (value) => { const settings = globalSettingsSchema.parse({ collapseCommandOutputByDefault: value }) diff --git a/packages/types/src/__tests__/index.test.ts b/packages/types/src/__tests__/index.test.ts index 15441d48fd..b4cee22f8c 100644 --- a/packages/types/src/__tests__/index.test.ts +++ b/packages/types/src/__tests__/index.test.ts @@ -3,6 +3,10 @@ import { GLOBAL_STATE_KEYS } from "../index.js" describe("GLOBAL_STATE_KEYS", () => { + it("should contain registered durable per-view state", () => { + expect(GLOBAL_STATE_KEYS).toContain("viewStates") + }) + it("should contain provider settings keys", () => { expect(GLOBAL_STATE_KEYS).toContain("autoApprovalEnabled") }) @@ -13,6 +17,7 @@ describe("GLOBAL_STATE_KEYS", () => { it("should not contain secret state keys", () => { expect(GLOBAL_STATE_KEYS).not.toContain("openRouterApiKey") + expect(GLOBAL_STATE_KEYS).not.toContain("apiKey") }) it("should contain OpenAI Compatible base URL setting", () => { diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index f5757a51c7..38d4292adb 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -114,6 +114,17 @@ export const MAX_CHECKPOINT_TIMEOUT_SECONDS = 60 */ export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15 +/** + * Persisted non-secret selections for a stable webview instance. + */ +export const viewStateSchema = z.object({ + mode: z.string().optional(), + currentApiConfigName: z.string().optional(), + updatedAt: z.number().optional(), + /** Host-minted token binding this entry to the webview that wrote it. */ + claimToken: z.string().optional(), +}) + /** * GlobalSettings */ @@ -122,6 +133,7 @@ export const globalSettingsSchema = z.object({ currentApiConfigName: z.string().optional(), listApiConfigMeta: z.array(providerSettingsEntrySchema).optional(), pinnedApiConfigs: z.record(z.string(), z.boolean()).optional(), + viewStates: z.record(z.string(), viewStateSchema).optional(), lastShownAnnouncementId: z.string().optional(), customInstructions: z.string().optional(), diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 8a006c0639..d24fd795e5 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -113,7 +113,14 @@ export interface ExtensionMessage { | "rooHistoryImportProgress" | "themeFixtureProbeRequest" | "diagnosticsRequest" + | "viewStateClaim" text?: string + /** + * For `viewStateClaim`: the durable `viewStates` key the delivered claim governs. + */ + viewStateId?: string + /** For `viewStateClaim`: the host-minted token the webview stores and echoes on launch. */ + viewStateClaim?: string /** For fileContent: { path, content, error? } */ fileContent?: { path: string; content: string | null; error?: string } /** For originalContent: the pre-edit file content of the requested tool message (null when unavailable) */ @@ -712,6 +719,9 @@ export interface WebviewMessage { | "themeFixtureProbeResponse" | "diagnosticsResponse" text?: string + viewStateId?: string + /** For `webviewDidLaunch`: the claim delivered for `viewStateId`, enforced by setViewStateId. */ + viewStateClaim?: string taskId?: string editedMessageContent?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud" diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index e53f60fb83..0d0a13e082 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -67,17 +67,22 @@ const globalSettingsExportSchema = globalSettingsSchema.omit({ taskHistory: true, listApiConfigMeta: true, currentApiConfigName: true, + // Per-view selection state is machine-local: it keeps flowing through the + // normal runtime and pruning paths but must not transfer between settings. + viewStates: true, }) export class ContextProxy { private readonly originalContext: vscode.ExtensionContext private stateCache: GlobalState + private stateWriteGenerations = new Map() private secretCache: SecretState private _isInitialized = false constructor(context: vscode.ExtensionContext) { this.originalContext = context + this.stateCache = {} this.secretCache = {} this._isInitialized = false @@ -397,6 +402,15 @@ export class ContextProxy { */ private readonly stateWriteOwner = new Map() private readonly secretWriteOwner = new Map() + /** + * The secret writes that are still in flight, each paired with the claim it took, + * so a reset can wait for them to settle before it deletes the keys. Without the + * pairing a reset cannot tell a settled-but-orphaned write from a live one. + */ + private readonly pendingSecretWrites = new Map }>>() + // A reset is in progress: the caches are meaningless mid-reset, and a secret write begun + // inside it would commit after the delete pass and survive the erase. + private resetting = false /** How many writes of each secret were started. Unlike the owners, it is not cleared when a write is over. */ private readonly secretWriteCount = new Map() @@ -413,6 +427,8 @@ export class ContextProxy { // writes that are in flight. An update that throws at once is turned into a rejection, so it is handled the same. const owner = {} this.stateWriteOwner.set(key, owner) + const generation = (this.stateWriteGenerations.get(key) ?? 0) + 1 + this.stateWriteGenerations.set(key, generation) this.stateCache[key] = value const release = () => { if (this.stateWriteOwner.get(key) === owner) this.stateWriteOwner.delete(key) @@ -429,6 +445,11 @@ export class ContextProxy { }) } + /** Current per-key write generation, used by callers whose rollback must not clobber a newer write. */ + public getStateWriteGeneration(key: K): number { + return this.stateWriteGenerations.get(key) ?? 0 + } + private getAllGlobalState(): GlobalState { return Object.fromEntries(GLOBAL_STATE_KEYS.map((key) => [key, this.getGlobalState(key)])) } @@ -443,6 +464,13 @@ export class ContextProxy { } storeSecret(key: SecretStateKey, value?: string) { + if (this.resetting) { + // Refused rather than deferred: a deferred secret write would re-store exactly the + // value the user asked to erase, and a silently dropped one would lose the save. + return Promise.reject( + new SettingsWriteError([], [key], new Error("secret writes are refused while state is being reset")), + ) + } // Update cache. const owner = {} this.secretWriteOwner.set(key, owner) @@ -454,7 +482,19 @@ export class ContextProxy { // cache (it stays empty until the next write, `refreshSecrets` or a restart) rather than being served. // The entry of the key is removed when the write and its read-back are over (if it is still the latest). A store // that throws at once is turned into a rejection, so it is handled the same. + const pending = this.pendingSecretWrites.get(key) ?? new Set<{ settle: Promise }>() + let settleWrite: () => void = () => {} + const entry: { settle: Promise } = { + settle: new Promise((resolve) => { + settleWrite = resolve + }), + } + pending.add(entry) + this.pendingSecretWrites.set(key, pending) const release = () => { + pending.delete(entry) + if (pending.size === 0) this.pendingSecretWrites.delete(key) + settleWrite() if (this.secretWriteOwner.get(key) === owner) this.secretWriteOwner.delete(key) } const stored = (async () => @@ -716,17 +756,42 @@ export class ContextProxy { * @returns A promise that resolves when all reset operations are complete */ public async resetAllState() { - // Clear in-memory caches - this.stateCache = {} - this.secretCache = {} + this.resetting = true + try { + // A secret write that is in flight at this moment has already committed nothing, but it will + // commit as soon as it settles, and the deletes below cannot un-commit a write that lands + // after them. Wait for the pending writes first, then end the claims, then delete, so the + // cleared copy is the last one to land in storage and the cache stays empty afterwards. + const inFlight = [...this.pendingSecretWrites.values()].flatMap((writes) => + [...writes].map((w) => w.settle), + ) + await Promise.allSettled(inFlight) + + // Clear in-memory caches + this.stateCache = {} + this.secretCache = {} + // A write that started before this reset must not restore its pre-reset value when it + // rejects afterwards, so advance every generation the pending writes still hold. + for (const key of this.stateWriteGenerations.keys()) { + this.stateWriteGenerations.set(key, (this.stateWriteGenerations.get(key) ?? 0) + 1) + } + // A reset also ends write ownership: a write that is still in flight reads the Memento back + // into the cache it just lost, so clearing the caches is not enough to keep pre-reset state + // out. Ownership is what the rejection handlers gate on. + this.stateWriteOwner.clear() + this.secretWriteOwner.clear() - await Promise.all([ - ...GLOBAL_STATE_KEYS.map((key) => this.originalContext.globalState.update(key, undefined)), - ...SECRET_STATE_KEYS.map((key) => this.originalContext.secrets.delete(key)), - ...GLOBAL_SECRET_KEYS.map((key) => this.originalContext.secrets.delete(key)), - ]) + await Promise.all([ + ...GLOBAL_STATE_KEYS.map((key) => this.originalContext.globalState.update(key, undefined)), + ...SECRET_STATE_KEYS.map((key) => this.originalContext.secrets.delete(key)), + ...GLOBAL_SECRET_KEYS.map((key) => this.originalContext.secrets.delete(key)), + ]) - await this.initialize() + await this.initialize() + this.resetting = false + } finally { + this.resetting = false + } } private static _instance: ContextProxy | null = null diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 01695d7994..07a5fc3d21 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -55,6 +55,14 @@ export const providerProfilesSchema = z.object({ export type ProviderProfiles = z.infer +/** Already-deleted profile: callers branch on this type instead of matching message text. */ +export class ProviderSettingsNotFoundError extends Error { + constructor(message: string) { + super(message) + this.name = "ProviderSettingsNotFoundError" + } +} + export class ProviderSettingsManager { private static readonly SCOPE_PREFIX = "roo_cline_config_" private readonly defaultConfigId = this.generateId() @@ -511,7 +519,7 @@ export class ProviderSettingsManager { const providerProfiles = await this.load() if (!providerProfiles.apiConfigs[name]) { - throw new Error(`Config '${name}' not found`) + throw new ProviderSettingsNotFoundError(`Config '${name}' not found`) } if (Object.keys(providerProfiles.apiConfigs).length === 1) { @@ -522,6 +530,11 @@ export class ProviderSettingsManager { await this.store(providerProfiles) }) } catch (error) { + // A missing config is a caller-meaningful signal, not a failure: rethrow it + // unwrapped so callers can branch on the type instead of message text. + if (error instanceof ProviderSettingsNotFoundError) { + throw error + } throw new Error(`Failed to delete config: ${error}`) } } diff --git a/src/core/config/__tests__/ContextProxy.spec.ts b/src/core/config/__tests__/ContextProxy.spec.ts index 5d78e16351..f9e9fc6a73 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -121,6 +121,188 @@ describe("ContextProxy", () => { expect(result).toBe("deepseek") }) + it("should not let an older rejected write clobber a newer cached value", async () => { + let rejectFirst: (error: Error) => void = () => {} + const firstWriteResult = new Promise((_, reject) => { + rejectFirst = reject + }) + + mockGlobalState.update.mockReset() + mockGlobalState.update.mockReturnValueOnce(firstWriteResult) + mockGlobalState.update.mockResolvedValueOnce(undefined) + + const firstWrite = proxy.updateGlobalState("apiProvider", "anthropic") + await proxy.updateGlobalState("apiProvider", "openai") + + // The older write rejects after a newer value already reached the cache: rolling + // the cache back to the pre-first-write value would diverge from storage. + rejectFirst(new Error("disk full")) + await expect(firstWrite).rejects.toThrow("disk full") + + expect(proxy.getGlobalState("apiProvider")).toBe("openai") + }) + + it("should not restore a pre-reset value when a pending write rejects after reset", async () => { + let rejectPending: (error: Error) => void = () => {} + mockGlobalState.update.mockReset() + mockGlobalState.update.mockReturnValueOnce( + new Promise((_, reject) => { + rejectPending = reject + }), + ) + const pending = proxy.updateGlobalState("apiProvider", "anthropic") + await proxy.resetAllState() + // The Memento answers the read-back with the pre-reset value: only ending write ownership + // during the reset keeps the cache cleared. With the mocked store this is the difference + // between the guarantee holding and the rejected write repopulating the cache. + mockGlobalState.get.mockImplementation((key: string) => (key === "apiProvider" ? "anthropic" : undefined)) + rejectPending(new Error("disk full")) + await expect(pending).rejects.toThrow("disk full") + expect(proxy.getGlobalState("apiProvider")).toBeUndefined() + }) + it("does not restore a pre-reset secret when a pending storeSecret settles after the reset", async () => { + const durable = new Map() + let settle: () => void = () => {} + mockSecrets.get.mockImplementation(async (key: string) => durable.get(key)) + mockSecrets.delete.mockImplementation(async (key: string) => { + durable.delete(key) + }) + mockSecrets.store.mockImplementationOnce( + (key: string, value: string) => + new Promise((resolve) => { + settle = () => { + durable.set(key, value) + resolve(undefined) + } + }), + ) + + const pending = proxy.storeSecret("apiKey", "pre-reset-secret") + setTimeout(settle, 0) + await proxy.resetAllState() + await pending + // The pending store is held open on a double that only puts the value into storage + // when it settles, so the ordering is observable: the reset must wait for the write to + // settle and then delete the keys, so neither the cache nor the durable copy keeps the + // value that was in flight when the reset started. + expect(await mockSecrets.get("apiKey")).toBeUndefined() + expect(proxy.getSecret("apiKey")).toBeUndefined() + }) + + // The delete pass is a single Promise.all over every key, so one keychain delete rejecting + // rejects resetAllState itself. Until now the only path on which the flag that guards + // storeSecret is cleared was the success path, so a single failed delete left every later + // secret write refused for the life of the instance. What is asserted below is that the + // store is usable again - the next save resolving and reading back through both the cache + // and storage - not merely that no error appears. + it("clears the reset guard when a secret delete rejects so the next save succeeds and reads back", async () => { + const durable = new Map() + mockSecrets.get.mockImplementation(async (key: string) => durable.get(key)) + mockSecrets.store.mockImplementation(async (key: string, value: string) => { + durable.set(key, value) + }) + mockSecrets.delete.mockImplementation(async (key: string) => { + if (key === "apiKey") { + throw new Error("keychain locked") + } + durable.delete(key) + }) + + // The original error is what propagates, not a wrapper: the caller has to be able to tell a + // failed erase from a refused write. + await expect(proxy.resetAllState()).rejects.toThrow("keychain locked") + + const after = proxy.storeSecret("apiKey", "written-after-failed-reset") + await expect(after).resolves.toBeUndefined() + expect(await mockSecrets.get("apiKey")).toBe("written-after-failed-reset") + expect(proxy.getSecret("apiKey")).toBe("written-after-failed-reset") + }) + + it("adopts a secret write issued after a reset that waited for a pending write", async () => { + const durable = new Map() + let settle: () => void = () => {} + mockSecrets.get.mockImplementation(async (key: string) => durable.get(key)) + mockSecrets.delete.mockImplementation(async (key: string) => { + durable.delete(key) + }) + mockSecrets.store.mockImplementationOnce( + (key: string, value: string) => + new Promise((resolve) => { + settle = () => { + durable.set(key, value) + resolve(undefined) + } + }), + ) + + const pending = proxy.storeSecret("apiKey", "pre-reset-secret") + const reset = proxy.resetAllState() + setTimeout(settle, 0) + await pending + await reset + + // The reset released only its own claims, so the machinery stays usable afterwards: + // a write issued once the reset has settled is adopted into the cache and reaches storage. + // Back the post-reset write with the durable double as well, so the assertion below reads + // storage rather than only the cache. + mockSecrets.store.mockImplementationOnce(async (key: string, value: string) => { + durable.set(key, value) + }) + await proxy.storeSecret("apiKey", "newer-secret") + expect(proxy.getSecret("apiKey")).toBe("newer-secret") + expect(await mockSecrets.get("apiKey")).toBe("newer-secret") + }) + + it("erases a secret whose write begins after the reset has snapshotted the pending writes", async () => { + const durable = new Map() + let deleteCalls = 0 + let settleFirstDelete: () => void = () => {} + let markDeletePassStarted: () => void = () => {} + const deletePassStarted = new Promise((resolve) => { + markDeletePassStarted = resolve + }) + mockSecrets.get.mockImplementation(async (key: string) => durable.get(key)) + mockSecrets.store.mockImplementation(async (key: string, value: string) => { + durable.set(key, value) + }) + // Park the reset inside its own delete pass so a write can begin in the window the in-flight + // snapshot cannot cover. Only the first delete is held open; the rest settle normally, so + // releasing the park lets the reset finish. + mockSecrets.delete.mockImplementation((key: string) => { + deleteCalls++ + durable.delete(key) + if (deleteCalls === 1) { + return new Promise((resolve) => { + markDeletePassStarted() + settleFirstDelete = () => resolve(undefined) + }) + } + return Promise.resolve(undefined) + }) + + const reset = proxy.resetAllState() + await deletePassStarted + + // The reset has taken its in-flight snapshot, cleared both caches and dropped both write + // claims, and is standing inside the delete pass. This write is invisible to all of it. + const late = proxy.storeSecret("apiKey", "late-secret") + settleFirstDelete() + await reset + await late.catch(() => undefined) + + // All three reads matter. Durable storage is what a reset is supposed to empty, the cache is + // what this provider instance shows the user, and re-initializing is what a restarted + // provider does - which is how a secret that survived the reset reaches the user after the + // fact, so the middle read alone would understate the finding. + expect(await mockSecrets.get("apiKey")).toBeUndefined() + expect(proxy.getSecret("apiKey")).toBeUndefined() + await proxy.initialize() + expect(proxy.getSecret("apiKey")).toBeUndefined() + // The caller is told: a write the reset cannot honour is refused rather than dropped, so + // the save surfaces as a failure instead of vanishing. + await expect(late).rejects.toThrow() + }) + it("should bypass cache for pass-through state keys", async () => { // Setup mock return value mockGlobalState.get.mockReturnValue("pass-through-value") @@ -1170,4 +1352,20 @@ Output only the summary of the conversation so far, without any additional comme expect(customSupportPromptsUpdateCalls.length).toBe(0) }) }) + + describe("export", () => { + it("should exclude viewStates from the exported settings", async () => { + await proxy.setValue("viewStates", { + "stable-sidebar-view": { mode: "architect", currentApiConfigName: "profile-a", updatedAt: 1 }, + }) + await proxy.setValue("customInstructions", "global instructions") + + const exported = await proxy.export() + + // Per-view selection state is machine-local and must never transfer + // between settings, while ordinary global settings keep round-tripping. + expect(exported).not.toHaveProperty("viewStates") + expect(exported?.customInstructions).toBe("global instructions") + }) + }) }) diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index a1fc9ac7de..c17c241fa3 100644 --- a/src/core/config/__tests__/ProviderSettingsManager.spec.ts +++ b/src/core/config/__tests__/ProviderSettingsManager.spec.ts @@ -12,7 +12,7 @@ import { import { clearAllMocks } from "../../../test-utils/reset" import { makeExtensionContext } from "../../../test-utils/vscode" -import { ProviderSettingsManager, ProviderProfiles } from "../ProviderSettingsManager" +import { ProviderSettingsManager, ProviderSettingsNotFoundError, ProviderProfiles } from "../ProviderSettingsManager" // `export()` builds an API handler per profile to read model capabilities. Mock // buildApiHandler with the real @roo-code/types model definitions so the token-field @@ -973,11 +973,27 @@ describe("ProviderSettingsManager", () => { }), ) + // The typed not-found signal is the contract callers branch on: a profile + // name containing "not found" must not be matchable via message text. + await expect(providerSettingsManager.deleteConfig("nonexistent")).rejects.toBeInstanceOf( + ProviderSettingsNotFoundError, + ) await expect(providerSettingsManager.deleteConfig("nonexistent")).rejects.toThrow( "Config 'nonexistent' not found", ) }) + it('keeps a storage failure whose message mentions "not found" out of the typed not-found signal', async () => { + mockSecrets.get.mockRejectedValue(new Error("vault: secret not found")) + + // A message-based classifier would treat this as a missing profile. The typed + // signal must stay reserved for a missing config entry. + await expect(providerSettingsManager.deleteConfig("default")).rejects.not.toBeInstanceOf( + ProviderSettingsNotFoundError, + ) + await expect(providerSettingsManager.deleteConfig("default")).rejects.toThrow("Failed to delete config") + }) + it("should throw error when trying to delete last remaining config", async () => { mockSecrets.get.mockResolvedValue( JSON.stringify({ diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 6c66655325..fca6cfd3c0 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -331,6 +331,50 @@ describe("importExport", () => { ]) }) + it("should not apply imported viewStates to the context proxy", async () => { + const fileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "test", + apiConfigs: { + test: { apiProvider: providerIdentifiers.openai, apiKey: "test-key", id: "test-id" }, + }, + }, + globalSettings: { + mode: "code", + viewStates: { + "stable-sidebar-view": { + mode: "architect", + currentApiConfigName: "profile-a", + updatedAt: 1, + }, + }, + }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(fileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } }, + }) + + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "test", id: "test-id", apiProvider: providerIdentifiers.openai }, + { name: "default", id: "default-id", apiProvider: providerIdentifiers.anthropic }, + ]) + + const result = await importSettingsFromPath("/mock/path/settings.json", { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + expect(result.success).toBe(true) + // Importing must not apply another machine's view pins. + expect(mockContextProxy.setValues).toHaveBeenCalledWith({ mode: "code" }) + expect(result).not.toHaveProperty("globalSettings.viewStates") + }) + it("should return success: false when file content is invalid", async () => { ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 9882039ad3..530e29cbb9 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -98,6 +98,12 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): { for (const [key, rawValue] of Object.entries(rawGlobalSettings)) { const path = `globalSettings.${key}` + + // Per-view selections are machine-local: they must not import across machines. + if (key === "viewStates") { + continue + } + const schema = globalSettingsShape[key as keyof GlobalSettings] if (!schema) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 87c37cf596..93971e1fad 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -74,6 +74,8 @@ import { isRetiredProvider, modelIdKeys, providerIdentifiers, + PROVIDER_SETTINGS_KEYS, + isSecretStateKey, } from "@roo-code/types" import { RateLimitClock, createRateLimitClock } from "../task/RateLimitClock" import { TaskRegistry } from "../task/TaskRegistry" @@ -160,6 +162,15 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" +type PersistedViewState = NonNullable[string] +type ViewLocalStateValues = Partial & Partial +/** + * The subset of view-local values that has a durable per-view record. `saveViewState` + * and the mutation path handle exactly these keys, so the generic stays narrow enough + * for the compiler to reject e.g. `writeDelayMs`. + */ +type PersistedViewStateValues = Pick + /** * https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts * https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts @@ -248,6 +259,12 @@ export class ClineProvider public static readonly sideBarId = `${Package.name}.SidebarProvider` public static readonly tabPanelId = `${Package.name}.TabPanelProvider` private static activeInstances: Set = new Set() + private static nextViewId = 0 + private static readonly MAX_PERSISTED_VIEW_STATES = 50 + private static readonly MAX_VIEW_STATE_ID_LENGTH = 64 + private static persistedViewStateWriteQueue: Promise = Promise.resolve() + // A durable view-state write that started before a reset must not persist after the clear. + private static viewStatesResetGeneration = 0 private webviewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< string, @@ -415,6 +432,38 @@ export class ClineProvider */ private clineMessagesSeq = 0 + /** + * Unique identifier for this provider instance's view. + * Based on renderContext and a monotonically increasing counter to ensure uniqueness across multiple instances. + */ + public readonly viewId: string + + /** Stable id for persisted per-view state keys, bound by the host-minted claim. */ + private viewStateId: string + + /** + * Whether this instance wrote an entry under its own session-local temporary id. + * Temporary ids restart from 0 on every extension host start, so an entry under the + * same name can belong to a different session. Only re-key entries this instance wrote. + */ + private wroteUnderTemporaryViewStateId = false + + /** + * Local state buffer for this specific view instance. + * Used to isolate mode, apiConfiguration, and other fields from the shared ContextProxy singleton + * when running in parallel (multi-tab) mode. + */ + private viewLocalState: Partial = {} + + /** + * This view's pinned profile name from the view-local buffer. Exposed for + * sibling-instance inspection (getAllInstances() filtering): `viewLocalState` + * is private and must not be reached through bracket access. + */ + get pinnedProfileName(): string | undefined { + return this.viewLocalState.currentApiConfigName + } + public isViewLaunched = false public settingsImportedAt?: number public readonly latestAnnouncementId = "oct-2026-v3.94.0-models-oauth-chat" // v3.94.0 model support, MCP OAuth, and chat reliability @@ -430,6 +479,10 @@ export class ClineProvider mdmService?: MdmService, ) { super() + // Initialize viewId based on renderContext and monotonically increasing instance identifier for uniqueness. + // activeInstances is used for visibility/iteration checks, so we keep tracking instances separately. + this.viewId = `${renderContext}-${ClineProvider.nextViewId++}` + this.viewStateId = this.viewId this.currentWorkspacePath = getWorkspacePath() this.pendingEditOperations = new PendingEditOperationStore( ClineProvider.PENDING_OPERATION_TIMEOUT_MS, @@ -463,6 +516,8 @@ export class ClineProvider await this.postStateToWebviewWithoutClineMessages() }) + void this.loadViewState() + // Initialize MCP Hub through the singleton manager McpServerManager.getInstance(this.context, this) .then((hub) => { @@ -599,6 +654,451 @@ export class ClineProvider } } + /** + * Reads the registered viewStates map, returning a defensive copy. + * When fresh is set, the map is read directly from globalState (bypassing the + * ContextProxy cache) so serialized writes never observe a stale in-memory value. + */ + private getPersistedViewStates(options: { fresh?: boolean } = {}): Record { + const viewStates = options.fresh + ? this.context.globalState.get("viewStates") + : this.contextProxy.getValue("viewStates") + + if (!viewStates || typeof viewStates !== "object" || Array.isArray(viewStates)) { + return Object.create(null) + } + + // Copy onto a null prototype. A view id that names an Object.prototype key + // ("constructor", "toString", "valueOf", ...) must not resolve through the + // prototype chain and be mistaken for a persisted entry. + return Object.assign(Object.create(null), viewStates) + } + + /** Persist the non-secret selections through the serialized write queue. */ + private async savePersistedViewState(values: Partial): Promise { + let landed = true + const resetGeneration = ClineProvider.viewStatesResetGeneration + // Capture the id at change time: a write belongs to the view that was active + // when the change was made, even if a newer id is registered while it is queued. + const viewStateId = this.viewStateId + // Set by the queued write when it had to mint a claim, delivered after it lands. + let mintedClaimToken: string | undefined + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + // The reset cleared viewStates while this write was queued: storing the pre-reset + // value now would resurrect it on the next reload. + if (ClineProvider.viewStatesResetGeneration !== resetGeneration) { + // The caller must not apply the in-memory half of a mutation storage rejected. + landed = false + return + } + + const states = this.getPersistedViewStates({ fresh: true }) + // The first write to an unowned temporary id claims the slot instead of merging + // into it: temporary names restart from 0 on every host start, so an entry under + // this name can be a leftover from an earlier session, and merging would adopt its + // fields and later re-key them under the stable id. Entries under a stable id, or + // under the temporary id this instance already authored, keep the merge semantics. + const ownsTargetEntry = viewStateId !== this.viewId || this.wroteUnderTemporaryViewStateId + const current = ownsTargetEntry ? (states[viewStateId] ?? {}) : {} + const next: PersistedViewState = { ...current } + + if ("mode" in values) { + if (values.mode === undefined || values.mode === null) { + delete next.mode + } else { + next.mode = values.mode + } + } + + if ("currentApiConfigName" in values) { + if (values.currentApiConfigName === undefined || values.currentApiConfigName === null) { + delete next.currentApiConfigName + } else { + next.currentApiConfigName = values.currentApiConfigName + } + } + + let wroteEntry = false + if (!next.mode && !next.currentApiConfigName) { + delete states[viewStateId] + } else { + next.updatedAt = values.updatedAt ?? Date.now() + // Bind the entry to this webview. An existing entry keeps the token it was + // adopted with (so a reload can still prove ownership); a new entry - or one + // written before claims existed - gets a fresh host-minted token. The token is + // never taken from `values`: it is host-owned, and webview input must not be + // able to choose or supply one. + if (!next.claimToken) { + next.claimToken = crypto.randomUUID() + mintedClaimToken = next.claimToken + } + states[viewStateId] = next + wroteEntry = true + } + + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states, viewStateId)) + // Authorship is recorded only after the durable write lands: a failed persistence + // must not mark the temporary id as owned, or a later load would adopt an entry + // that was never actually stored. + if (wroteEntry && viewStateId === this.viewId) { + this.wroteUnderTemporaryViewStateId = true + } + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + + // Delivered only once the durable write landed: a webview must never hold a claim the + // host did not store, or the next launch would present a token the entry cannot match. + if (mintedClaimToken) { + this.postViewStateClaimToWebview(viewStateId, mintedClaimToken) + } + + return landed + } + + /** + * Removes the given view's entry from the registered viewStates map. + * Runs through the serialized write queue to avoid racing concurrent view-state writes. + */ + private async clearPersistedViewState(viewStateId = this.viewStateId): Promise { + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + delete states[viewStateId] + await this.contextProxy.setValue("viewStates", states) + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** Re-bind a claim-less entry with a fresh claim; unowned selections are dropped. */ + private async rebindUnboundViewStateEntry(viewStateId: string): Promise { + const claimToken = crypto.randomUUID() + let rebound = false + + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + const current = states[viewStateId] + // A concurrent registration may have bound this key while this write was queued: + // never overwrite an existing claim, and let the caller refuse the stale adoption. + if (current && typeof current.claimToken === "string" && current.claimToken) { + return + } + states[viewStateId] = { claimToken, updatedAt: Date.now() } + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states, viewStateId)) + rebound = true + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + + return rebound ? claimToken : undefined + } + + /** + * Delivers the host-minted claim that governs a durable viewStates key to this view's + * webview. The webview only stores it and echoes it back on the next launch: it can + * never mint or choose a claim itself. + */ + private postViewStateClaimToWebview(viewStateId: string, claimToken: string): void { + void this.postMessageToWebview({ type: "viewStateClaim", viewStateId, viewStateClaim: claimToken }) + } + + /** + * Keeps only the most recently updated entries of the persisted view states map, + * bounded by MAX_PERSISTED_VIEW_STATES so the global key cannot grow unboundedly. + * + * `protectedKey` is the entry the calling writer just stored. Retention ranks entries by + * `updatedAt`, and a wall clock that moved backwards - a corrected system clock, a VM + * restored to an earlier time - leaves the whole retained window stamped later than + * `Date.now()`, so the entry written right now ranks below the limit and would be dropped + * while the write still reports success. The protected entry takes the oldest retained slot + * instead; it sorts below every retained entry by construction, so the ordering of the + * others is untouched. + */ + private prunePersistedViewStates( + states: Record, + protectedKey?: string, + ): Record { + const entries = Object.entries(states) + const retained = entries + .sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)) + .slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES) + + if (protectedKey !== undefined && !retained.some(([key]) => key === protectedKey)) { + const protectedEntry = entries.find(([key]) => key === protectedKey) + + if (protectedEntry) { + retained[retained.length - 1] = protectedEntry + } + } + + return Object.fromEntries(retained) + } + + /** Re-key this instance's authored pre-launch entry to the stable id. */ + private async rekeyPersistedViewStateEntry(nextViewStateId: string): Promise { + const previousViewStateId = this.viewId + + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + // An entry under this name can belong to a different session, because temporary + // ids restart from 0 on every extension host start. Only re-key entries this + // instance wrote, otherwise a stale entry from another session is adopted. + if (!this.wroteUnderTemporaryViewStateId) { + return + } + + const states = this.getPersistedViewStates({ fresh: true }) + const previous = states[previousViewStateId] + + if (!previous) { + return + } + + delete states[previousViewStateId] + + const existingStableEntry = states[nextViewStateId] + + if (!existingStableEntry) { + states[nextViewStateId] = previous + } else { + // Registration can bind a claim-less legacy key just before this pre-launch entry + // is re-keyed. Keep the selections the stable entry already carries, but fill in + // anything it lacks so a mode chosen before launch is not silently dropped. + states[nextViewStateId] = { + ...previous, + ...existingStableEntry, + updatedAt: Math.max(previous.updatedAt ?? 0, existingStableEntry.updatedAt ?? 0), + } + } + + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states, nextViewStateId)) + + // Only a successful write clears authorship: a failed attempt leaves the entry + // under the temporary name, and a retry must still be able to re-key it. + this.wroteUnderTemporaryViewStateId = false + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** Register the stable view id and load its persisted selections; a claim must match. */ + public async setViewStateId(viewStateId: string | undefined, presentedClaim?: string): Promise { + // The webview generates this id on purpose - it has to survive a webview reload, + // which recreates the view on both sides - so the host cannot mint it instead. + // What the host owns is which view may hold a given persisted key, so everything + // below validates the supplied value before it becomes one. + if (typeof viewStateId !== "string") { + // A crafted postMessage can carry any JSON value here even though the message + // type declares a string; the persisted key must stay a string. + this.log(`[setViewStateId] Ignoring a non-string viewStateId for view ${this.viewId}.`) + return + } + + const normalizedViewStateId = viewStateId.trim().replace(/[^A-Za-z0-9_-]/g, "_") + + if ( + !normalizedViewStateId || + normalizedViewStateId === this.viewStateId || + // Reject "__proto__" and any other prototype-chain key. The map is copied + // onto a null prototype, so these names can no longer resolve through the + // chain, but they are still rejected as ids so a caller cannot store an + // entry under a name that looks like an inherited member. + normalizedViewStateId === "__proto__" + ) { + return + } + + // Bound the key: the webview generates a UUID, so a much longer value is not a + // view id, and an unbounded key is unbounded growth in the shared viewStates map. + if (normalizedViewStateId.length > ClineProvider.MAX_VIEW_STATE_ID_LENGTH) { + this.log( + `[setViewStateId] Ignoring an oversized viewStateId (${normalizedViewStateId.length} chars) for view ${this.viewId}.`, + ) + return + } + + const claimant = ClineProvider.getAllInstances().find( + (instance) => instance !== this && instance.viewStateId === normalizedViewStateId, + ) + if (claimant) { + // Ownership check: a key another live view holds must not be adopted. Doing so + // would load that view's pinned profile, nested overlay, and mode into this + // provider, and this webview could then overwrite that view's persisted entry. + // Staying on the temporary id keeps this view working against the shared state. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: already claimed by live view ${claimant.viewId}.`, + ) + return + } + + // Claim binding, checked before any re-key or load so a refused registration can + // neither read nor mutate a persisted entry. Read fresh from storage rather than + // through the ContextProxy cache: a security check must not trust a stale copy. + const persistedEntry = this.getPersistedViewStates({ fresh: true })[normalizedViewStateId] + + if (persistedEntry) { + const storedClaim = persistedEntry.claimToken + + if (typeof storedClaim !== "string" || !storedClaim) { + // A pre-claim entry cannot be attributed to any webview, so its selections are + // never loaded. Re-establish the binding with a fresh host-minted claim and hand + // that claim to this webview: the key becomes adoptable again (no upgrade brick) + // and the unbound selections are gone, so the new token cannot resurrect them. + const reboundClaim = await this.rebindUnboundViewStateEntry(normalizedViewStateId) + + if (reboundClaim === undefined) { + // A concurrent registration bound the key while the re-bind was queued: this + // launch adopts a key another webview now owns, so refuse it instead of + // handing out a claim that is not ours. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: bound concurrently by another registration.`, + ) + return + } + + this.postViewStateClaimToWebview(normalizedViewStateId, reboundClaim) + this.log( + `[setViewStateId] Re-bound claim-less viewStates entry ${normalizedViewStateId} for view ${this.viewId}: unbound selections were not loaded.`, + ) + } else if (typeof presentedClaim !== "string" || presentedClaim !== storedClaim) { + // Fail closed: the entry belongs to a webview that never presented this view's + // claim. Rejection is side-effect free, so presenting an id cannot probe, mutate + // or delete another view's entry. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: claim missing or mismatched.`, + ) + return + } + } + + const previousViewStateId = this.viewStateId + + this.viewStateId = normalizedViewStateId + + try { + // Re-key any durable entry written under the temporary pre-launch id before + // loading, so the load sees the view's own pre-registration selections. + await this.rekeyPersistedViewStateEntry(this.viewStateId) + + await this.loadViewState() + + // Re-affirm the binding for the id just adopted: a claim minted while this view was + // still on its temporary id now governs this key (the entry was re-keyed), so the + // webview is told the canonical (viewStateId, claim) pair to echo on its next launch. + const adoptedClaim = this.getPersistedViewStates({ fresh: true })[this.viewStateId]?.claimToken + + if (adoptedClaim) { + this.postViewStateClaimToWebview(this.viewStateId, adoptedClaim) + } + } catch (error) { + // A persistence failure must not leave the provider holding an id that was + // never registered: restore the previous id so a later launch retries the + // registration and the load instead of the guard above early-returning for + // the failed id. + this.viewStateId = previousViewStateId + throw error + } + } + + /** Load non-secret persisted selections; in-flight buffer edits are reapplied on top. */ + private async loadViewState(): Promise { + // A newer id registered while this load is in flight must win. + const loadedForViewId = this.viewStateId + + // Before registration the key is the session-local name, which can be a stale entry + // from another session: load only entries this instance authored. + if (loadedForViewId === this.viewId && !this.wroteUnderTemporaryViewStateId) { + return + } + try { + const persisted = this.getPersistedViewStates()[loadedForViewId] + const loadedState: Partial = {} + + // The mutation paths update viewLocalState in place; a shallow snapshot makes + // in-flight edits observable below. + const preLoadBuffer = { ...this.viewLocalState } + + if (persisted?.mode) { + // A stale custom-mode slug must not shadow the shared mode from getState(). + const customModes = await this.customModesManager.getCustomModes() + if (getModeBySlug(persisted.mode, customModes)) { + loadedState.mode = persisted.mode as Mode + } else { + this.log(`[loadViewState] Ignoring unknown persisted mode "${persisted.mode}"`) + } + } + + if (persisted?.currentApiConfigName) { + loadedState.currentApiConfigName = persisted.currentApiConfigName + + try { + const { name: _name, ...apiConfiguration } = await this.providerSettingsManager.getProfile({ + name: persisted.currentApiConfigName, + }) + loadedState.apiConfiguration = apiConfiguration as ProviderSettings + } catch (error) { + this.log( + `[loadViewState] Unable to resolve API profile '${persisted.currentApiConfigName}' for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + + if (this.viewStateId !== loadedForViewId) { + this.log(`[loadViewState] Discarding stale state for superseded view id ${loadedForViewId}`) + return + } + + // Diff each field so in-flight edits win over the loaded values (including clears) + // while untouched fields keep the persisted values authoritative. + const postLoadBuffer = this.viewLocalState + const mergedState: Partial = { ...loadedState } + + if (!Object.is(preLoadBuffer.mode, postLoadBuffer.mode)) { + if (postLoadBuffer.mode === undefined) { + delete mergedState.mode + } else { + mergedState.mode = postLoadBuffer.mode + } + } + + if (!Object.is(preLoadBuffer.currentApiConfigName, postLoadBuffer.currentApiConfigName)) { + if (postLoadBuffer.currentApiConfigName === undefined) { + delete mergedState.currentApiConfigName + } else { + mergedState.currentApiConfigName = postLoadBuffer.currentApiConfigName + } + } + + if (!Object.is(preLoadBuffer.apiConfiguration, postLoadBuffer.apiConfiguration)) { + if (postLoadBuffer.apiConfiguration === undefined) { + delete mergedState.apiConfiguration + } else { + mergedState.apiConfiguration = postLoadBuffer.apiConfiguration + } + } + + this.viewLocalState = mergedState + this.log(`[loadViewState] Loaded state for viewId ${this.viewId}`) + } catch (error) { + this.log( + `[loadViewState] Error loading state for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + + /** Save one view-local value; the durable subset is re-keyed to the stable id. */ + public async saveViewState( + key: K, + value: PersistedViewStateValues[K] | undefined, + ): Promise { + await this._saveViewLocalStateFromMutation({ [key]: value } as ViewLocalStateValues) + + this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`) + } + /** * Override EventEmitter's on method to match TaskProviderLike interface */ @@ -2354,20 +2854,170 @@ export class ClineProvider // We should probably switch to that and verify that it works. // I left the original implementation in just to be safe. // `allSettled` so a caller that rolls back after a failure never races a write still in flight. + // Capture every store the activation group rewrites: one member can reject after + // others landed, and restoring only the shared name would leave the old name next to + // the new settings and a mode binding pointing at the new profile. + const previousModeConfigId = this.providerSettingsManager.getModeConfigId + ? await this.providerSettingsManager.getModeConfigId(mode) + : undefined + // Snapshot every store after the last await above: another view can write any of them while + // this activation is suspended, and a snapshot taken earlier would both mis-arm the + // ownership generations below and restore a value that was never current. + const previousList = this.contextProxy.getValue("listApiConfigMeta") + const previousName = this.contextProxy.getValue("currentApiConfigName") + const previousViewName = this.viewLocalState.currentApiConfigName + // The buffered settings belong to the profile this view was running under. Restoring only the + // name would leave the failed profile's settings pinned to the restored name, which is the + // mismatched pair getValues() serves from the overlay. + const previousViewApiConfiguration = this.viewLocalState.apiConfiguration + // This activation owns the shared selection until a newer write bumps the generation. + const expectedNameGeneration = this.contextProxy.getStateWriteGeneration("currentApiConfigName") + 1 + // The profile list needs its own ownership signal: a non-activating save from another view + // writes listApiConfigMeta without touching the shared name, and undoing it would drop the + // profile that save just stored from the metadata. + const expectedListGeneration = this.contextProxy.getStateWriteGeneration("listApiConfigMeta") + 1 + const previousProviderSettings = this.contextProxy.getProviderSettings() + // Ownership signal for the provider-settings store, independent of the shared name: + // this activation's `setProviderSettings` writes `apiProvider` exactly once, so those + // settings are still ours while that key holds the generation the write produced. + const expectedApiProviderGeneration = this.contextProxy.getStateWriteGeneration("apiProvider") + 1 + // Test doubles and older managers may not implement the read: absence means "no previous + // binding to restore", not a failed activation. + const activationResults = await Promise.allSettled([ this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()), - this.updateGlobalState("currentApiConfigName", name), + // Shared write: adds/updates the profile list entry and the current-profile name. + // It goes straight to ContextProxy rather than through this.setValue: the per-view mirror + // inside setValue restores the shared name itself when this view's write rejects, and that + // restore advances the shared-selection generation this activation is tracked against, so the + // rollback below would read its own undo as another view's write and skip restoring this + // activation's mode binding, leaving the mode on the profile whose activation failed. + this.contextProxy.setValue("currentApiConfigName", name), this.providerSettingsManager.setModeConfig(mode, id), this.contextProxy.setProviderSettings(providerSettings), + // setProviderSettings bypasses the buffer path; the mutation below refreshes this view. + // One mutation carries both the name and the settings: pinnedProfileName reads the buffer, + // so a buffer that keeps the previous name while holding the new settings pins a profile + // this view no longer runs under. + this._saveViewLocalStateFromMutation({ + currentApiConfigName: name, + apiConfiguration: providerSettings, + }), ]) const failedActivation = activationResults.find( (result): result is PromiseRejectedResult => result.status === "rejected", ) - if (failedActivation) throw failedActivation.reason + if (failedActivation) { + // The mutation queue is per view, but ContextProxy is shared: another view may have + // activated a profile between the group and this rollback. Every store is undone + // only while this activation still owns it, and each store has its own signal - + // gating all of them on the shared-name generation would skip undoing this + // activation's mode binding and settings whenever another view changed only the + // shared name, which leaves the failed activation's values in place. + // * profile list + shared selection: the generation of the shared name, + // * mode binding: the binding still points at this activation's profile id, + // * shared provider settings: the generation of `apiProvider`, which this group + // writes exactly once. + const ownsSharedSelection = () => + this.contextProxy.getStateWriteGeneration("currentApiConfigName") === expectedNameGeneration + + const restoreFailures: string[] = [] + const restore = async (label: string, action: () => Promise) => { + try { + await action() + } catch (restoreError) { + restoreFailures.push( + `${label}: ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`, + ) + } + } + await restore("profile list", async () => { + if (this.contextProxy.getStateWriteGeneration("listApiConfigMeta") !== expectedListGeneration) { + this.log( + "[persistProviderProfile] Skipping the profile-list restore: another view wrote the profile list", + ) + return + } + await this.contextProxy.setValue("listApiConfigMeta", previousList) + }) + await restore("view-local selection", () => + this._saveViewLocalStateFromMutation({ + currentApiConfigName: previousViewName, + apiConfiguration: previousViewApiConfiguration, + }), + ) + await restore("mode binding", async () => { + // A newer activation from another view re-pointed this mode: its binding wins. + // When this activation's own binding write is what failed, the binding never + // moved and there is nothing to undo. + // + // The profile id alone cannot answer that: another view can activate the same + // profile, in which case the binding still reads as ours and restoring it would + // overwrite the other view's selection. The shared-selection generation is the + // ownership signal for "this write is still mine to undo". + if (!ownsSharedSelection()) { + this.log( + "[persistProviderProfile] Skipping the mode-binding restore: another view wrote the shared selection", + ) + return + } + const currentModeConfigId = this.providerSettingsManager.getModeConfigId + ? await this.providerSettingsManager.getModeConfigId(mode) + : previousModeConfigId + + if (currentModeConfigId !== id) { + this.log( + "[persistProviderProfile] Skipping the mode-binding restore: the mode no longer points at this activation's profile", + ) + return + } + + await this.providerSettingsManager.setModeConfig(mode, previousModeConfigId as string) + }) + await restore("shared provider settings", async () => { + if (this.contextProxy.getStateWriteGeneration("apiProvider") !== expectedApiProviderGeneration) { + this.log( + "[persistProviderProfile] Skipping the shared-settings restore: another view wrote the provider settings", + ) + return + } + + await this.contextProxy.setProviderSettings(previousProviderSettings) + }) + // Recheck immediately before the write: the restores above await storage, so + // another view may have written the shared selection since the group failed. + await restore("shared selection", async () => { + if (!ownsSharedSelection()) { + this.log( + "[persistProviderProfile] Skipping the shared-selection restore: another view wrote it", + ) + return + } + await this.contextProxy.setValue("currentApiConfigName", previousName) + }) + + if (restoreFailures.length > 0) { + const original = failedActivation.reason + const prefix = original instanceof Error ? original.message : String(original) + throw new Error(`${prefix}; activation rollback also failed: ${restoreFailures.join("; ")}`, { + cause: original instanceof Error ? original : undefined, + }) + } + + throw failedActivation.reason + } if (signal.aborted) return id + // setProviderSettings bypasses the buffer path: refresh this view's overlay only after + // every activation write landed, so a failed group cannot leave the overlay ahead of + // the restored shared state. + await this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }) + + // Refresh other views pinned to this profile so their overlays cannot go stale. + await this.refreshSiblingsAfterProfileSave(name, providerSettings) + // Change the provider for the current task. // TODO: We should rename `buildApiHandler` for clarity (e.g. `getProviderClient`). this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true }) @@ -2376,6 +3026,20 @@ export class ClineProvider await this.persistStickyProviderProfileToCurrentTask(name) } else { await this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()) + + // A non-activating save still changes the profile's settings: refresh the views + // that buffered a copy, or their getState() keeps the pre-save values. + if (this.pinnedProfileName === name) { + await this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }) + // Only rebuild a task that actually runs under the saved profile. + if (this.getCurrentTask()?.taskApiConfigName === name) { + this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true }) + } + } + // A non-activating save changes the profile's settings too: refresh every sibling + // that buffered a copy (pinned siblings included), or their getState() keeps the pre-save + // values. This head had no sibling refresh at all before this change. + await this.refreshSiblingsAfterProfileSave(name, providerSettings) } if (signal.aborted) return id @@ -2383,6 +3047,32 @@ export class ClineProvider await this.postStateToWebview() return id } + /** + * Refresh the buffered overlay of every live sibling pinned to a saved profile and rebuild + * any task that follows it. The handler rebuild is independent of the pin: a task restored + * from history can follow profile A while its view pins profile B. + */ + private async refreshSiblingsAfterProfileSave(name: string, providerSettings: ProviderSettings): Promise { + for (const instance of ClineProvider.getAllInstances()) { + if (instance === this || instance._disposed || instance.pinnedProfileName !== name) { + continue + } + // Direct private access: compile-time safe across sibling instances. + await instance._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }) + } + + const { + name: _recordName, + id: _recordId, + ...taskSettings + } = providerSettings as ProviderSettings & { name?: string; id?: string } + for (const instance of ClineProvider.getAllInstances()) { + if (instance._disposed || instance.getCurrentTask()?.taskApiConfigName !== name) { + continue + } + instance.updateTaskApiHandlerIfNeeded(taskSettings, { forceRebuild: true }) + } + } async deleteProviderProfile(profileToDelete: ProviderSettingsEntry) { const globalSettings = this.contextProxy.getValues() @@ -2472,11 +3162,67 @@ export class ClineProvider if (!skipCurrentTaskRebuild) { // See `upsertProviderProfile` for a description of what this is doing. + // Capture the shared values this group is about to replace: the per-view mirror below is the + // last step, and its rejection must not leave the shared selection and settings naming a + // profile the view buffer and the durable pin do not follow. + const sharedValuesBeforeActivation = { + list: this.contextProxy.getValue("listApiConfigMeta"), + name: this.contextProxy.getValue("currentApiConfigName"), + nameGeneration: this.contextProxy.getStateWriteGeneration("currentApiConfigName") + 1, + settings: this.contextProxy.getProviderSettings(), + apiProviderGeneration: this.contextProxy.getStateWriteGeneration("apiProvider") + 1, + } await Promise.all([ this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), this.contextProxy.setValue("currentApiConfigName", name), this.contextProxy.setProviderSettings(providerSettings), ]) + + // Mirror the activation into this view's buffer and durable pin: the shared writes above + // bypass the per-view mutation path, so getValues() would keep overlaying the previous + // profile's name and settings - getConfiguration() then reports the previous profile + // while the shared selection already holds the new one. + try { + await this._saveViewLocalStateFromMutation({ + currentApiConfigName: name, + apiConfiguration: providerSettings, + }) + } catch (mirrorError) { + // The shared writes already landed. Put them back while this activation still owns them, + // so the shared selection cannot name a profile the view buffer does not follow. + if ( + this.contextProxy.getStateWriteGeneration("currentApiConfigName") === + sharedValuesBeforeActivation.nameGeneration + ) { + await this.contextProxy + .setValue("currentApiConfigName", sharedValuesBeforeActivation.name) + .catch((restoreError: unknown) => + this.log( + `[activateProviderProfile] Failed to restore the shared selection after a rejected view mirror: ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`, + ), + ) + await this.contextProxy + .setValue("listApiConfigMeta", sharedValuesBeforeActivation.list) + .catch((restoreError: unknown) => + this.log( + `[activateProviderProfile] Failed to restore the profile list after a rejected view mirror: ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`, + ), + ) + } + if ( + this.contextProxy.getStateWriteGeneration("apiProvider") === + sharedValuesBeforeActivation.apiProviderGeneration + ) { + await this.contextProxy + .setProviderSettings(sharedValuesBeforeActivation.settings) + .catch((restoreError: unknown) => + this.log( + `[activateProviderProfile] Failed to restore the shared provider settings after a rejected view mirror: ${restoreError instanceof Error ? restoreError.message : String(restoreError)}`, + ), + ) + } + throw mirrorError + } } const { mode } = await this.getState() @@ -3707,15 +4453,79 @@ export class ClineProvider } public async setValue(key: K, value: RooCodeSettings[K]) { + const previousSharedValues = { [key]: this.contextProxy.getValue(key) } as RooCodeSettings + // This write produces the next generation for the key: only restore while it is still newest. + const writeGeneration = this.contextProxy.getStateWriteGeneration(key as never) + 1 await this.contextProxy.setValue(key, value) + try { + await this._saveViewLocalStateFromMutation({ [key]: value }) + } catch (error) { + // The per-view record did not land: restore the prior shared value so the two + // persistence layers cannot diverge, then surface the original failure. + await this._restoreSharedValuesAfterFailedViewWrite( + previousSharedValues, + { [key]: value } as RooCodeSettings, + { [key]: writeGeneration } as Record, + ) + throw error + } } public getValue(key: K) { return this.contextProxy.getValue(key) } - public getValues() { - return this.contextProxy.getValues() + /** + * This view's own values over the shared store. + * + * The buffered profile name and the settings that were loaded for it are overlaid as one + * unit. Overlaying the name on its own would hand a consumer this view's profile name + * carrying whichever profile the shared store currently holds. `loadViewState` records the + * name before it resolves the profile and keeps it when the lookup fails, so a buffered name + * with no settings behind it is overlaid only when the shared selection is that same profile - + * the case where the shared settings genuinely belong to it. Otherwise this view reads the + * shared name and settings pair until its profile resolves; it never gets one half of a pair. + */ + public getValues(): RooCodeSettings { + return { ...this.contextProxy.getValues(), ...this._viewLocalValuesOverlay() } + } + + /** The view buffer flattened onto the shared key set, ready to overlay `getValues()`. */ + private _viewLocalValuesOverlay(): RooCodeSettings { + const { mode, currentApiConfigName, apiConfiguration } = this.viewLocalState + const overlay: RooCodeSettings = {} + + if (mode !== undefined) { + overlay.mode = mode + } + + if (currentApiConfigName === undefined) { + return overlay + } + + if (apiConfiguration === undefined) { + if (currentApiConfigName === this.contextProxy.getValue("currentApiConfigName")) { + overlay.currentApiConfigName = currentApiConfigName + } + + return overlay + } + + overlay.currentApiConfigName = currentApiConfigName + + // Flattened with the same key set as `ContextProxy.getProviderSettings()`, and with the + // same exclusion: secrets live in the shared secret state, not in a per-view overlay. + // Every other provider key - including the ones this profile does not carry - comes from + // this view's profile, so the name above never travels with another profile's settings. + for (const key of PROVIDER_SETTINGS_KEYS) { + if (isSecretStateKey(key)) { + continue + } + + ;(overlay as Record)[key] = apiConfiguration[key] + } + + return overlay } public async setValues(values: RooCodeSettings) { @@ -3724,8 +4534,41 @@ export class ClineProvider ) } - // dev + /** Persist the durable subset first, then update the in-memory buffer. */ + private async _saveViewLocalStateFromMutation( + values: Partial & Partial, + ): Promise { + const landed = await this._persistViewLocalStateFromMutation(values) + if (!landed) { + // A reset that beat this write cleared the buffer; applying the value now would + // leave the view reporting a selection a reload drops. + return + } + this._updateViewLocalStateFromMutation(values) + } + + /** Write the durable subset of a mutation into this view's viewStates entry. */ + private async _persistViewLocalStateFromMutation( + values: Partial & Partial, + ): Promise { + const persistedValues: Partial = {} + + if ("mode" in values) { + persistedValues.mode = values.mode as PersistedViewState["mode"] + } + + if ("currentApiConfigName" in values) { + persistedValues.currentApiConfigName = values.currentApiConfigName + } + + if ("mode" in persistedValues || "currentApiConfigName" in persistedValues) { + return this.savePersistedViewState(persistedValues) + } + + return true + } + /** Clear the buffer so getState() falls back to the shared values. */ async resetState() { const answer = await vscode.window.showInformationMessage( t("common:confirmation.reset_state"), @@ -3749,7 +4592,17 @@ export class ClineProvider } } + // A view-state write queued before the reset would otherwise read the cleared map and store + // its pre-reset entry after resetAllState(): drain the queue first, then bump the + // generation so a write that started before this point is rejected inside the queue. + await ClineProvider.persistedViewStateWriteQueue.catch(() => undefined) + ClineProvider.viewStatesResetGeneration += 1 await this.contextProxy.resetAllState() + + // The reset cleared the shared store: drop this view's buffered selections too, or + // getValues()/getConfiguration() keep serving pre-reset values. + this.viewLocalState = {} + await this.providerSettingsManager.resetAllConfigs() await this.customModesManager.resetCustomModes() await this.removeClineFromStack() @@ -5487,4 +6340,93 @@ export class ClineProvider return vscode.Uri.file(filePath).toString() } } + + /** + * Update or invalidate viewLocalState when ContextProxy is mutated via setValues, setValue, + * profile upsert/activation/deletion, or resetState. This ensures the local cache stays in + * sync with global state changes that would otherwise be invisible behind mergedStateValues. + */ + private _updateViewLocalStateFromMutation(values: Partial & Partial): void { + if ("mode" in values) { + const val = values.mode + if (val === undefined || val === null) { + delete this.viewLocalState.mode + } else { + this.viewLocalState.mode = val + } + } + + if ("currentApiConfigName" in values) { + const val = values.currentApiConfigName + if (val === undefined || val === null) { + delete this.viewLocalState.currentApiConfigName + } else { + this.viewLocalState.currentApiConfigName = val + } + } + + if ("apiConfiguration" in values) { + const val = values.apiConfiguration + if (val === undefined || val === null) { + delete this.viewLocalState.apiConfiguration + } else { + this.viewLocalState.apiConfiguration = val + } + } else if (PROVIDER_SETTINGS_KEYS.some((key) => key in values)) { + const providerSettingsUpdate = PROVIDER_SETTINGS_KEYS.reduce((acc, key) => { + if (key in values) { + return { ...acc, [key]: values[key as keyof RooCodeSettings] } + } + + return acc + }, {} as ProviderSettings) + + this.viewLocalState.apiConfiguration = + "apiProvider" in providerSettingsUpdate + ? providerSettingsUpdate + : { + ...(this.viewLocalState.apiConfiguration ?? {}), + ...providerSettingsUpdate, + } + } + } + + /** + * Restore shared values captured before a setter wrote them, used when the paired + * per-view persistence failed so the shared store cannot move ahead of the per-view + * record. A failed rollback is logged; the original failure is what callers see. + */ + private async _restoreSharedValuesAfterFailedViewWrite( + previous: RooCodeSettings, + attempted: RooCodeSettings, + generations: Record = {}, + ): Promise { + // Roll back only the keys this attempt wrote and that still hold the value it wrote. + // A concurrent provider may have written a newer value in the meantime - even the same + // value - so the per-key write generation must also still match; restoring otherwise + // would clobber that newer write for every view that reads shared state. + const rollback: RooCodeSettings = {} as RooCodeSettings + + for (const key of Object.keys(attempted) as (keyof RooCodeSettings)[]) { + const generation = generations[key] + if ( + this.contextProxy.getValue(key) === attempted[key] && + (generation === undefined || this.contextProxy.getStateWriteGeneration(key as never) === generation) + ) { + ;(rollback as Record)[key] = previous[key] + } + } + + if (Object.keys(rollback).length === 0) { + return + } + + try { + await this.contextProxy.setValues(rollback) + } catch (rollbackError) { + this.log( + `[ClineProvider] Shared-state rollback after failed per-view write failed: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + ) + } + } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 9826fc206b..1a3447fcea 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -71,14 +71,15 @@ vi.mock("fs/promises", async (importOriginal) => { } }) -vi.mock("axios", async (importOriginal) => ({ +vi.mock("axios", () => ({ default: { - isAxiosError: (await importOriginal()).default.isAxiosError, get: vi.fn().mockResolvedValue({ data: { data: [] } }), post: vi.fn(), + isAxiosError: vi.fn().mockReturnValue(true), }, get: vi.fn().mockResolvedValue({ data: { data: [] } }), post: vi.fn(), + isAxiosError: vi.fn().mockReturnValue(true), })) vi.mock("../../../utils/safeWriteJson") @@ -425,6 +426,31 @@ afterAll(() => { vi.restoreAllMocks() }) +type StalledProfile = { + name: string + apiProvider?: string + openRouterModelId?: string +} + +function stallProviderSettingsProfile(provider: ClineProvider) { + let resolveProfile: (value: StalledProfile) => void = () => {} + const getProfileSpy = vi.fn(() => { + if (getProfileSpy.mock.calls.length > 1) { + throw new Error( + "stallProviderSettingsProfile: getProfile called more than once; only one lookup is resolvable", + ) + } + return new Promise((resolve) => (resolveProfile = resolve)) + }) + // @ts-ignore - Reassign the readonly providerSettingsManager for the test; the double only backs getProfile. + provider.providerSettingsManager = { getProfile: getProfileSpy } + return { getProfile: getProfileSpy, resolveProfile: (value: StalledProfile) => resolveProfile(value) } +} + +function persistedViewStateClaim(context: vscode.ExtensionContext, viewStateId: string): string | undefined { + return context.globalState.get("viewStates")?.[viewStateId]?.claimToken +} + describe("ClineProvider", () => { beforeAll(() => { vi.mocked(Task).mockImplementation(function (options: any) { @@ -822,7 +848,6 @@ describe("ClineProvider", () => { } }, ) - test("background messages reach the provider handler without changing the last focused chat", async () => { sidebar.messages.fire({ type: "webviewDidFocus" }) const handler = vi @@ -3051,7 +3076,6 @@ describe("ClineProvider", () => { }) }) }) - test("resolveWebviewView sets up webview correctly in development mode even if local server is not running", async () => { const developmentContext = { ...mockContext, extensionMode: vscode.ExtensionMode.Development } provider = new ClineProvider( @@ -3101,7 +3125,6 @@ describe("ClineProvider", () => { expect(html).toContain("") expect(html).not.toContain("/@react-refresh") }) - test("resolveWebviewView builds HMR content against the local dev server when it is reachable", async () => { const originalThemeFixtureProbe = process.env.ROO_CODE_THEME_FIXTURE_PROBE delete process.env.ROO_CODE_THEME_FIXTURE_PROBE @@ -5719,6 +5742,1769 @@ describe("ClineProvider", () => { ]) }) }) + + describe("setViewStateId", () => { + it('should ignore "__proto__" and keep the temporary viewId', async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["setViewStateId"]("__proto__") + + // "__proto__" is rejected before assignment so a per-view entry can never be + // keyed through the Object.prototype setter: the temporary id stays active and + // nothing is persisted under the reserved name. + expect(provider["viewStateId"]).toBe(provider.viewId) + expect(mockContext.globalState.get("viewStates")).toBeUndefined() + expect(provider["viewLocalState"]).toEqual({}) + + await provider.dispose() + }) + + it("restores the previous view id when the registration write fails so a later launch retries", async () => { + const contextProxy = new ContextProxy(mockContext) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + // Seed a pre-launch entry under the temporary id through the real write path, + // so the re-key has real work to do and records this instance as its author. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + + const setValueSpy = vi.spyOn(contextProxy, "setValue").mockRejectedValue(new Error("storage down")) + + await expect(provider["setViewStateId"]("stable-sidebar-view")).rejects.toThrow("storage down") + + // The failed id must not stick: the provider keeps its previous (temporary) + // id so a later launch retries registration and the load instead of the + // guard early-returning for an id that was never persisted. + expect(provider["viewStateId"]).toBe(provider.viewId) + + // Pin what the failure left in each store: the durable record still keys the entry under + // the temporary id (the re-key wrote nothing durable), and the payload ContextProxy holds for + // viewStates is the same map the durable store holds, so a rolled-back registration cannot be + // visible through one store and invisible through the other. + const durableAfterFailure = mockContext.globalState.get("viewStates") as Record + expect(durableAfterFailure[provider.viewId]?.mode).toBe("architect") + expect(durableAfterFailure["stable-sidebar-view"]).toBeUndefined() + expect(provider.contextProxy.getValue("viewStates")).toEqual(durableAfterFailure) + + // A later retry succeeds once the storage write works again, and the + // pre-launch entry lands under the registered id. + setValueSpy.mockRestore() + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewStateId"]).toBe("stable-sidebar-view") + expect(mockContext.globalState.get("viewStates")).toEqual({ + "stable-sidebar-view": { mode: "architect", updatedAt: 1, claimToken: expect.any(String) }, + }) + + await provider.dispose() + }) + }) + + describe("view state persistence across a reset", () => { + it("drops a queued view-state write when a reset clears viewStates, including the buffer half", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["setViewStateId"]("stable-sidebar-view") + + // Hold the shared write queue so the save below is still queued when the reset + // generation advances, exactly like resetState() racing a save from the webview. + let releaseQueue!: () => void + const heldQueue = new Promise((resolve) => { + releaseQueue = resolve + }) + ClineProvider["persistedViewStateWriteQueue"] = heldQueue + + const queuedSave = provider.saveViewState("mode", "ask") + // resetState() clears the durable map after bumping the generation (resetAllState + // then wipes the cache); the generation guard must reject the queued write. + ClineProvider["viewStatesResetGeneration"] += 1 + await provider.contextProxy.setValue("viewStates", undefined) + + releaseQueue() + await queuedSave + + // The write started before the reset: storage stays cleared and the buffer does + // not keep a mode that a reload would drop. + expect(mockContext.globalState.get("viewStates")).toBeUndefined() + expect(provider["viewLocalState"].mode).toBeUndefined() + + await provider.dispose() + }) + }) + + describe("view state claim binding", () => { + it("refuses a viewStateId bound by a concurrent registration through setViewStateId", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const beforeViewStateId = provider["viewStateId"] + const postSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + // A claim-less entry is adoptable at the first read, but a concurrent registration + // binds the key while this launch's queued re-bind is waiting its turn. + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + await provider["setViewStateId"]("legacy-view") + + // The launch must not adopt a key another webview now owns: no claim is delivered, + // the provider stays on its temporary id, and the winner's claim is untouched. + expect(postSpy).not.toHaveBeenCalledWith(expect.objectContaining({ type: "viewStateClaim" })) + expect(provider["viewStateId"]).toBe(beforeViewStateId) + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + postSpy.mockRestore() + await provider.dispose() + }) + + it("does not overwrite a claim bound by a concurrent registration", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + // A concurrent registration binds this key while the re-bind write is queued. + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + const claim = await provider["rebindUnboundViewStateEntry"]("legacy-view") + + // The queued re-bind must not replace the claim the other registration wrote; it + // reports that nothing was rebound so the caller can refuse the stale adoption. + expect(claim).toBeUndefined() + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + await provider.dispose() + }) + + const createProvider = () => + new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const storedEntries = () => + mockContext.globalState.get>("viewStates") ?? {} + + test("refuses a disposed view's id when no claim is presented, and loads nothing", async () => { + // The vulnerable path: a view registers a durable entry, is disposed, and a second + // webview presents the same id. dispose() removes the instance but leaves the entry, + // so only the claim can stop the second webview from loading the first one's pinned + // profile (and the profile's secrets) through the state post. + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("mode", "architect") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + + const storedClaim = storedEntries()["closed-view"]?.claimToken + expect(storedClaim).toEqual(expect.any(String)) + + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + await intruder["setViewStateId"]("closed-view") + + // Fail closed: the id is not adopted, no selections reach this view and no state is + // posted back to the webview that asked for it. + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect((await intruder.getState()).currentApiConfigName).not.toBe("profile-with-secrets") + expect(postMessageSpy).not.toHaveBeenCalled() + + // A refused adoption is side-effect free: presenting an id must not probe or rewrite + // the entry that a different webview owns. + expect(storedEntries()["closed-view"]).toMatchObject({ + mode: "architect", + currentApiConfigName: "profile-with-secrets", + claimToken: storedClaim, + }) + + await intruder.dispose() + }) + + test("refuses a mismatched claim", async () => { + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + // A webview-generated value is not a claim: only the token the host minted matches. + await intruder["setViewStateId"]("closed-view", "webview-chosen-claim") + + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect(postMessageSpy).not.toHaveBeenCalled() + + await intruder.dispose() + }) + + test("does not churn the claim on later writes to the same entry", async () => { + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + await provider["setViewStateId"]("minted-view") + await provider.saveViewState("mode", "architect") + + const mintedClaim = storedEntries()["minted-view"]?.claimToken + expect(mintedClaim).toEqual(expect.any(String)) + + await provider.saveViewState("currentApiConfigName", "profile-b") + + // A merge into an entry this view already owns keeps its token, so the claim the + // webview holds keeps working and is not re-sent. + expect(storedEntries()["minted-view"]?.claimToken).toBe(mintedClaim) + expect( + postMessageSpy.mock.calls.map(([message]) => message).filter((m) => m.type === "viewStateClaim"), + ).toHaveLength(1) + }) + + test("re-binds a claim-less legacy entry instead of loading its selections", async () => { + // An entry written before claims existed cannot be attributed to any webview: its + // selections are refused, and the key is re-bound with a fresh host-minted claim so + // the upgrade does not brick per-view persistence. + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { mode: "architect", currentApiConfigName: "profile-a", updatedAt: 1 }, + }) + + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + + await provider["setViewStateId"]("legacy-view") + + // No stored selection is adopted... + expect(provider["viewStateId"]).toBe("legacy-view") + expect(provider["viewLocalState"]).toStrictEqual({}) + + // ...the old entry is replaced by a claim-bearing one with the unbound selections gone... + const rebound = storedEntries()["legacy-view"] + expect(rebound).toEqual({ claimToken: expect.any(String), updatedAt: expect.any(Number) }) + expect(rebound).not.toHaveProperty("mode") + expect(rebound).not.toHaveProperty("currentApiConfigName") + + // ...and the fresh claim is handed to this webview so the next launch can prove ownership. + expect(postMessageSpy).toHaveBeenCalledWith({ + type: "viewStateClaim", + viewStateId: "legacy-view", + viewStateClaim: rebound?.claimToken, + }) + + await provider.dispose() + + const reopened = createProvider() + await reopened["setViewStateId"]("legacy-view", rebound?.claimToken) + + expect(reopened["viewStateId"]).toBe("legacy-view") + expect(reopened["viewLocalState"]).toStrictEqual({}) + + await reopened.dispose() + }) + + test("still refuses an id a live view holds, even when the claim matches", async () => { + const owner = createProvider() + await owner["setViewStateId"]("shared-live-view") + await owner.saveViewState("mode", "architect") + const claim = storedEntries()["shared-live-view"]?.claimToken + + const sibling = createProvider() + + try { + await sibling["setViewStateId"]("shared-live-view", claim) + + // The live-instance guard runs first and keeps working: a second live view never + // shares a key with the view that holds it. + expect(sibling["viewStateId"]).toBe(sibling.viewId) + expect(sibling["viewLocalState"]).toStrictEqual({}) + expect(owner["viewStateId"]).toBe("shared-live-view") + } finally { + await sibling.dispose() + await owner.dispose() + } + }) + }) + + describe("view state persistence edge cases", () => { + it("should read viewStates from the ContextProxy cache when not fresh", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { "stable-sidebar-view": { mode: "architect" } }) + expect(provider["getPersistedViewStates"]()).toEqual({ "stable-sidebar-view": { mode: "architect" } }) + await provider.dispose() + }) + + it("should treat a corrupted non-object viewStates value as an empty map", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await mockContext.globalState.update("viewStates", "corrupted-storage-value") + expect(provider["getPersistedViewStates"]({ fresh: true })).toEqual({}) + await provider.dispose() + }) + + it("should merge saved fields, drop cleared fields and delete emptied entries", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + const save = provider.saveViewState.bind(provider) as (key: string, value: unknown) => Promise + const states = () => mockContext.globalState.get>("viewStates") ?? {} + await provider["setViewStateId"]("stable-sidebar-view") + await provider.saveViewState("mode", "architect") + await provider.saveViewState("currentApiConfigName", "profile-a") + const merged = states()["stable-sidebar-view"] + expect(merged).toMatchObject({ mode: "architect", currentApiConfigName: "profile-a" }) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Saved mode for viewId")) + await save("mode", undefined) + expect(states()["stable-sidebar-view"]).toStrictEqual({ + currentApiConfigName: "profile-a", + updatedAt: expect.any(Number), + claimToken: expect.any(String), + }) + await save("mode", null) + expect(states()["stable-sidebar-view"]).not.toHaveProperty("mode") + await provider.saveViewState("mode", "architect") + await save("currentApiConfigName", undefined) + expect(states()["stable-sidebar-view"]).toStrictEqual({ + mode: "architect", + updatedAt: expect.any(Number), + claimToken: expect.any(String), + }) + await provider.saveViewState("currentApiConfigName", "profile-c") + await provider.saveViewState("mode", "architect") + expect(states()["stable-sidebar-view"]).toMatchObject({ + mode: "architect", + currentApiConfigName: "profile-c", + }) + await save("currentApiConfigName", null) + expect(states()["stable-sidebar-view"]).not.toHaveProperty("currentApiConfigName") + await save("mode", null) + expect(states()["stable-sidebar-view"]).toBeUndefined() + expect(provider["viewLocalState"]).toStrictEqual({}) // buffer ends fully cleared + await provider.dispose() + }) + + it("should rekey a pre-launch entry under the temporary id to the registered stable id", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // Seed through the write path so this instance authors the entry under its temporary id; an entry authored by another session must not be adopted. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + await provider["setViewStateId"]("stable-sidebar-view") + expect(mockContext.globalState.get("viewStates")).toEqual({ + // The minted claim travels with the re-keyed entry, so the webview's token keeps working. + "stable-sidebar-view": { mode: "architect", updatedAt: 1, claimToken: expect.any(String) }, + }) + await provider.dispose() + }) + + it("should keep the stable entry and drop the temporary entry when both exist", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // Author the temporary entry through the write path so this instance owns it, + // then add a stable entry that already exists in storage. + await provider["savePersistedViewState"]({ mode: "temp-mode", updatedAt: 1 }) + await provider.contextProxy.setValue("viewStates", { + [provider.viewId]: { mode: "temp-mode", updatedAt: 1 }, + "stable-sidebar-view": { mode: "stable-mode", updatedAt: 5, claimToken: "stable-claim" }, + }) + await provider["setViewStateId"]("stable-sidebar-view", "stable-claim") + expect(mockContext.globalState.get("viewStates")).toEqual({ + "stable-sidebar-view": { mode: "stable-mode", updatedAt: 5, claimToken: "stable-claim" }, + }) + await provider.dispose() + }) + + it("should prune by updatedAt regardless of insertion order", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const states = Object.fromEntries( + Array.from({ length: 55 }, (_, index) => [ + `view-${index}`, + { mode: `mode-${index}`, updatedAt: (index * 7) % 55 }, + ]), + ) + const pruned = provider["prunePersistedViewStates"](states) + expect(Object.keys(pruned)).toHaveLength(50) + // view-1/view-54 survive the true newest-50 selection; view-8 (updatedAt 1) does not. + expect(pruned["view-1"]).toEqual({ mode: "mode-1", updatedAt: 7 }) + expect(pruned["view-54"]).toEqual({ mode: "mode-54", updatedAt: 48 }) + expect(pruned["view-8"]).toBeUndefined() + await provider.dispose() + }) + + it("should log a successful empty load when no persisted entry exists", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewLocalState"]).toEqual({}) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Loaded state for viewId")) + await provider.dispose() + }) + + it("should keep the persisted profile name and log when the profile lookup fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + // @ts-ignore - Replace providerSettingsManager with a failing test double. + provider.providerSettingsManager = { getProfile: vi.fn().mockRejectedValue(new Error("profile missing")) } + await provider.saveViewState("currentApiConfigName", "my-profile") + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile") + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Unable to resolve API profile 'my-profile'")) + await provider.dispose() + }) + + it("should discard a stale load when the viewStateId changes during the profile lookup", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + // @ts-ignore - Replace providerSettingsManager with a test double that registers a newer id. + provider.providerSettingsManager = { + getProfile: vi.fn().mockImplementation(() => { + provider["viewStateId"] = "superseded-view" + return Promise.resolve({ name: "my-profile", apiProvider: providerIdentifiers.openrouter }) + }), + } + await provider.saveViewState("currentApiConfigName", "my-profile") + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + const staleMsg = expect.stringContaining("Discarding stale state for superseded view id") + expect(logSpy).toHaveBeenCalledWith(staleMsg) + await provider.dispose() + }) + + it("should not restore a persisted mode whose custom mode no longer exists", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + // @ts-ignore - Replace customModesManager with a test double (no custom modes). + provider.customModesManager = { getCustomModes: vi.fn().mockResolvedValue([]), dispose: vi.fn() } + // The file-level modes mock resolves every slug to a mode; drop the pinned slug. + const modesModule = vi.mocked(await import("../../../shared/modes")) + const originalMode = modesModule.getModeBySlug("code") + modesModule.getModeBySlug.mockImplementation(((slug: string) => + slug === "deleted-custom-mode" ? undefined : originalMode) as typeof modesModule.getModeBySlug) + try { + await provider["setViewStateId"]("stable-sidebar-view") + await provider.saveViewState("mode", "deleted-custom-mode") + // Reload the persisted entry: the custom mode was deleted in the meantime. + await provider["loadViewState"]() + // The stale slug must not be restored into the buffer. + expect(provider["viewLocalState"]).not.toHaveProperty("mode") + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('Ignoring unknown persisted mode "deleted-custom-mode"'), + ) + } finally { + modesModule.getModeBySlug.mockReturnValue(originalMode) + } + await provider.dispose() + }) + + it("should keep every persisted field authoritative when the load is untouched", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const { getProfile: getProfileSpy, resolveProfile } = stallProviderSettingsProfile(provider) + // @ts-ignore - Replace customModesManager with a test double (no custom modes). + provider.customModesManager = { getCustomModes: vi.fn().mockResolvedValue([]), dispose: vi.fn() } + await provider.saveViewState("mode", "code") + await provider.saveViewState("currentApiConfigName", "cfg-a") + const load = provider["setViewStateId"]("stable-sidebar-view") + + // Let the stalled lookup begin so the in-flight mutation and the resolver + // target the pending promise rather than the initial no-op. + await vi.waitFor(() => expect(getProfileSpy).toHaveBeenCalledTimes(1)) + // No mutation while the lookup is in flight: the persisted values must win. + resolveProfile({ name: "cfg-a", apiProvider: providerIdentifiers.openrouter, openRouterModelId: "model-x" }) + await load + + expect(provider["viewLocalState"]).toEqual({ + mode: "code", + currentApiConfigName: "cfg-a", + apiConfiguration: { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "model-x" }, + }) + await provider.dispose() + }) + // The activation path writes the profile settings straight into this view's buffer, so the + // buffered profile name has to move with them: pinnedProfileName reads the buffer, and a later + // non-activating save of the same profile is gated on that name. + it("keeps the buffered profile name in step with the profile it activates", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await saver["saveViewState"]("currentApiConfigName", "profile-a") + expect(saver["pinnedProfileName"]).toBe("profile-a") + // @ts-ignore - The double only backs the profile-store calls of the save path. + saver.providerSettingsManager = { + saveConfig: vi.fn().mockResolvedValue("profile-b-id"), + listConfig: vi.fn().mockResolvedValue([{ name: "profile-b", id: "profile-b-id" }]), + setModeConfig: vi.fn().mockResolvedValue(undefined), + } + const providerSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "activated-key" } + + await saver.upsertProviderProfile("profile-b", providerSettings, true) + + // One mutation: the name and the settings land in the buffer together, or the view pins a + // profile whose settings it is no longer holding. + expect(saver["viewLocalState"]).toMatchObject({ + currentApiConfigName: "profile-b", + apiConfiguration: providerSettings, + }) + expect(saver["pinnedProfileName"]).toBe("profile-b") + await saver.dispose() + }) + + it("should refresh the buffered overlay of another view pinned to the saved profile on a non-activating save", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const pinned = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // saveViewState is the buffer write on this head: setValue is a ContextProxy pass-through, + // so going through setValue here would leave pinnedProfileName undefined. + await pinned["saveViewState"]("currentApiConfigName", "shared-profile") + // @ts-ignore - The double only backs the profile-store calls of the save path. + saver.providerSettingsManager = { + saveConfig: vi.fn().mockResolvedValue("shared-id"), + listConfig: vi.fn().mockResolvedValue([{ name: "shared-profile", id: "shared-id" }]), + setModeConfig: vi.fn().mockResolvedValue(undefined), + } + const providerSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" } + + await saver.upsertProviderProfile("shared-profile", providerSettings, false) + + // The pinned view buffered a copy of the profile's settings; a non-activating save must + // refresh it, or its getState() keeps serving the pre-save configuration. + expect(pinned["viewLocalState"].apiConfiguration).toEqual(providerSettings) + await saver.dispose() + await pinned.dispose() + }) + + it("should refresh the buffered overlay of another view pinned to the activated profile", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const pinned = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // saveViewState is the buffer write on this head: setValue is a ContextProxy pass-through, + // so going through setValue here would leave pinnedProfileName undefined. + await pinned["saveViewState"]("currentApiConfigName", "shared-profile") + // @ts-ignore - The double only backs the profile-store calls of the save path. + saver.providerSettingsManager = { + saveConfig: vi.fn().mockResolvedValue("shared-id"), + listConfig: vi.fn().mockResolvedValue([{ name: "shared-profile", id: "shared-id" }]), + setModeConfig: vi.fn().mockResolvedValue(undefined), + } + const providerSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" } + + await saver.upsertProviderProfile("shared-profile", providerSettings, true) + + // The pinned view buffered a copy of the profile's settings; an activating save refreshes + // it through the sibling loop, so this row covers the activate=true branch that the case + // above no longer reaches. + expect(pinned["viewLocalState"].apiConfiguration).toEqual(providerSettings) + await saver.dispose() + await pinned.dispose() + }) + + // The task-handler rebuild is keyed on the task's own profile name rather than on the view's + // pin: a task restored from history can follow profile A while its view pins profile B, and only + // the matching side gets its handler rebuilt with forceRebuild (ClineProvider.ts:2897-2902). + // These two cases are the matching and non-matching halves of that branch, which is the coverage + // the review row on this file asked for. + it("rebuilds a sibling task's API handler with a forced rebuild when that task follows the saved profile", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const runner = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // The rebuild itself is stubbed so the case asserts the call and its options rather than the + // internals of a handler build. The assertion-free cast is needed because + // updateTaskApiHandlerIfNeeded is private on ClineProvider and vi.spyOn needs the object and + // the key together; the private-field alternative of instance["name"] cannot be spied. + const task = new Task(defaultTaskOptions) + Object.defineProperty(task, "taskApiConfigName", { value: "shared-profile" }) + vi.spyOn(runner, "getCurrentTask").mockReturnValue(task) + vi.spyOn(saver, "getCurrentTask").mockReturnValue(undefined) + const rebuildTarget = runner as unknown as { + updateTaskApiHandlerIfNeeded: ( + settings: Record, + options?: { forceRebuild?: boolean }, + ) => void + } + const rebuild = vi.spyOn(rebuildTarget, "updateTaskApiHandlerIfNeeded").mockImplementation(() => {}) + vi.spyOn(saver.providerSettingsManager, "saveConfig").mockResolvedValue("shared-id") + vi.spyOn(saver.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "shared-profile", id: "shared-id" }, + ]) + vi.spyOn(saver.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + + await saver.upsertProviderProfile( + "shared-profile", + { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" }, + false, + ) + + // Forced, because the saved settings can change with the provider and model both the same, + // and a rebuild that skipped on equality would leave the running task on the old client. + expect(rebuild).toHaveBeenCalledWith( + expect.objectContaining({ apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" }), + { forceRebuild: true }, + ) + expect(rebuild).toHaveBeenCalledTimes(1) + + await saver.dispose() + await runner.dispose() + vi.restoreAllMocks() + }) + + it("leaves a sibling task alone when that task follows a different profile than the one saved", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const runner = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // Same shape as the case above with one value changed: the sibling's task follows another + // profile, so its handler must keep the settings it was built with rather than inherit this + // save's settings. Its overlay refresh is a separate question and is covered by the two cases + // above this one; this case is only about the rebuild's own selector. + const task = new Task(defaultTaskOptions) + Object.defineProperty(task, "taskApiConfigName", { value: "other-profile" }) + vi.spyOn(runner, "getCurrentTask").mockReturnValue(task) + vi.spyOn(saver, "getCurrentTask").mockReturnValue(undefined) + const rebuildTarget = runner as unknown as { + updateTaskApiHandlerIfNeeded: ( + settings: Record, + options?: { forceRebuild?: boolean }, + ) => void + } + const rebuild = vi.spyOn(rebuildTarget, "updateTaskApiHandlerIfNeeded").mockImplementation(() => {}) + vi.spyOn(saver.providerSettingsManager, "saveConfig").mockResolvedValue("shared-id") + vi.spyOn(saver.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "shared-profile", id: "shared-id" }, + ]) + vi.spyOn(saver.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + + await saver.upsertProviderProfile( + "shared-profile", + { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" }, + false, + ) + + expect(rebuild).not.toHaveBeenCalled() + + await saver.dispose() + await runner.dispose() + vi.restoreAllMocks() + }) + }) + + describe("saveViewState", () => { + it("should update viewLocalState and persist mode through registered viewStates", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const contextProxySpy = vi.spyOn(provider.contextProxy, "setValue") + await provider["setViewStateId"]("stable-sidebar-view") + + await provider.saveViewState("mode", "architect") + + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider.contextProxy.getValue("viewStates")).toMatchObject({ + "stable-sidebar-view": { mode: "architect" }, + }) + expect(contextProxySpy).toHaveBeenCalledWith( + "viewStates", + expect.objectContaining({ + "stable-sidebar-view": expect.objectContaining({ + mode: "architect", + updatedAt: expect.any(Number), + }), + }), + ) + expect(contextProxySpy).not.toHaveBeenCalledWith("__view_state_stable-sidebar-view_mode", expect.anything()) + + await provider.dispose() + }) + + it("should update viewLocalState and persist currentApiConfigName through registered viewStates", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["setViewStateId"]("stable-sidebar-view") + await provider.saveViewState("currentApiConfigName", "my-profile") + + expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile") + expect(provider.contextProxy.getValue("viewStates")).toMatchObject({ + "stable-sidebar-view": { currentApiConfigName: "my-profile" }, + }) + + await provider.dispose() + }) + + it("should update viewLocalState for apiConfiguration without persisting provider settings or secrets", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const testApiConfig = { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "claude-3.5-sonnet", + openRouterApiKey: "secret-key", + } + + await provider["setViewStateId"]("stable-sidebar-view") + await provider.saveViewState("apiConfiguration", testApiConfig) + + expect(provider["viewLocalState"].apiConfiguration).toEqual(testApiConfig) + expect(provider.contextProxy.getValue("viewStates")).toBeUndefined() + + await provider.dispose() + }) + + it("should clear local override when saveViewState receives undefined", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider.saveViewState("mode", "architect") + expect(provider["viewLocalState"].mode).toBe("architect") + + await provider.saveViewState("mode", undefined) + + expect(Object.prototype.hasOwnProperty.call(provider["viewLocalState"], "mode")).toBe(false) + + await provider.dispose() + }) + + it("should clear the currentApiConfigName override when saveViewState receives undefined", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider.saveViewState("currentApiConfigName", "my-profile") + expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile") + + await provider.saveViewState("currentApiConfigName", undefined) + + expect(Object.prototype.hasOwnProperty.call(provider["viewLocalState"], "currentApiConfigName")).toBe(false) + + 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), + new WebviewFocusTracker(), + ) + vi.spyOn(provider.contextProxy, "setValue").mockRejectedValueOnce(new Error("persist failed")) + + await provider["setViewStateId"]("stable-sidebar-view") + + await expect(provider.saveViewState("mode", "architect")).rejects.toThrow("persist failed") + expect(provider["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, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + + new WebviewFocusTracker(), + ) + const provider2 = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider1["setViewStateId"]("stable-sidebar-view") + await provider2["setViewStateId"]("stable-editor-view") + + await Promise.all([ + provider1.saveViewState("mode", "architect"), + provider2.saveViewState("currentApiConfigName", "editor-profile"), + ]) + + expect(mockContext.globalState.get("viewStates")).toMatchObject({ + "stable-sidebar-view": { mode: "architect" }, + "stable-editor-view": { currentApiConfigName: "editor-profile" }, + }) + + await provider1.dispose() + await provider2.dispose() + }) + }) + + describe("loadViewState", () => { + it("should keep viewLocalState empty when no stable per-view values exist", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await vi.waitFor(() => { + expect(provider["viewLocalState"]).toEqual({}) + }) + + const state = await provider.getState() + // No per-view entry: getState() falls back to the shared defaults. + expect(state.mode).toBe("code") + expect(state.currentApiConfigName).toBe("default") + + await provider.dispose() + }) + + it("should log and keep existing viewLocalState when loadViewState fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + + // Register a stable id so the load is not skipped by the ownership guard: + // only entries this instance authored are loaded under a temporary name. + await provider["setViewStateId"]("stable-sidebar-view") + + provider["viewLocalState"] = { mode: "architect" } + vi.spyOn(provider.contextProxy, "getValue").mockImplementation(() => { + throw new Error("load failed") + }) + + await provider["loadViewState"]() + + expect(provider["viewLocalState"].mode).toBe("architect") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Error loading state")) + + await provider.dispose() + }) + }) + + describe("persisted view state pruning", () => { + it("should keep the newest 50 persisted view states", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const states = Object.fromEntries( + Array.from({ length: 55 }, (_, index) => [ + `view-${index}`, + { mode: `mode-${index}`, updatedAt: index }, + ]), + ) + + const pruned = provider["prunePersistedViewStates"](states) + + expect(Object.keys(pruned)).toHaveLength(50) + expect(pruned["view-54"]).toEqual({ mode: "mode-54", updatedAt: 54 }) + expect(pruned["view-5"]).toEqual({ mode: "mode-5", updatedAt: 5 }) + expect(pruned["view-4"]).toBeUndefined() + + await provider.dispose() + }) + }) + + describe("persisted view state pruning protection", () => { + // A wall clock that stepped backwards (a corrected system clock, a VM restored to an + // earlier time) leaves the whole retained window stamped later than `Date.now()`, so an + // entry written now ranks below the retention limit. + const futureStampedStates = (count: number) => + Object.fromEntries( + Array.from({ length: count }, (_, index) => [ + `future-view-${index}`, + { mode: "architect", updatedAt: Date.now() + 60_000 + index }, + ]), + ) + const storedViewStates = () => + mockContext.globalState.get>("viewStates") + + it("should keep the entry a save just wrote when its timestamp sorts below the retention limit", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider["setViewStateId"]("clock-skew-view") + await provider.contextProxy.setValue("viewStates", futureStampedStates(51)) + + const landed = await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + const states = storedViewStates() + + expect(landed).toBe(true) + expect(Object.keys(states ?? {})).toHaveLength(50) + expect(states?.["clock-skew-view"]).toMatchObject({ mode: "architect" }) + + await provider.dispose() + }) + + it("should keep the claim a rebind just minted when its timestamp sorts below the retention limit", async () => { + await mockContext.globalState.update("viewStates", { + ...futureStampedStates(51), + "legacy-view": { mode: "architect", updatedAt: 1 }, + }) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const claimToken = await provider["rebindUnboundViewStateEntry"]("legacy-view") + const states = storedViewStates() + + expect(claimToken).toEqual(expect.any(String)) + expect(Object.keys(states ?? {})).toHaveLength(50) + expect(states?.["legacy-view"]).toMatchObject({ claimToken }) + + await provider.dispose() + }) + + it("should keep the entry a re-key just moved when its timestamp sorts below the retention limit", async () => { + await mockContext.globalState.update("viewStates", futureStampedStates(51)) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // Authored under this instance's temporary id and stamped with the current time. + await provider.saveViewState("mode", "architect") + const temporaryViewStateId = provider["viewStateId"] + + await provider["rekeyPersistedViewStateEntry"]("clock-skew-stable-view") + const states = storedViewStates() + + expect(Object.keys(states ?? {})).toHaveLength(50) + expect(states?.["clock-skew-stable-view"]).toMatchObject({ mode: "architect" }) + expect(states?.[temporaryViewStateId]).toBeUndefined() + + await provider.dispose() + }) + }) + + describe("getValues view-local overlay", () => { + it("should return each view's own profile name with that profile's settings", async () => { + // One shared ContextProxy, as in the extension host: both views read the same store. + const sharedProxy = new ContextProxy(mockContext) + const sidebar = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + sharedProxy, + new WebviewFocusTracker(), + ) + const panel = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + sharedProxy, + new WebviewFocusTracker(), + ) + // The shared store carries whichever profile is active globally, as flattened keys. + await sidebar.contextProxy.setValue("openRouterModelId", "shared-model") + await sidebar.setValue("mode", "architect") + await sidebar.setValue("currentApiConfigName", "sidebar-profile") + await sidebar.saveViewState("apiConfiguration", { + apiProvider: providerIdentifiers.anthropic, + anthropicBaseUrl: "https://sidebar.example", + }) + await panel.setValue("mode", "code") + await panel.setValue("currentApiConfigName", "panel-profile") + await panel.saveViewState("apiConfiguration", { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "panel-model", + }) + + expect(sidebar.getValues().mode).toBe("architect") + expect(sidebar.getValues().currentApiConfigName).toBe("sidebar-profile") + expect(sidebar.getValues().apiProvider).toBe(providerIdentifiers.anthropic) + expect(sidebar.getValues().anthropicBaseUrl).toBe("https://sidebar.example") + // The other profile's flattened settings must not travel with this view's name. + expect(sidebar.getValues().openRouterModelId).toBeUndefined() + + expect(panel.getValues().mode).toBe("code") + expect(panel.getValues().currentApiConfigName).toBe("panel-profile") + expect(panel.getValues().apiProvider).toBe(providerIdentifiers.openrouter) + expect(panel.getValues().openRouterModelId).toBe("panel-model") + + await sidebar.dispose() + await panel.dispose() + }) + + it("should not pair a buffered profile name with the shared settings when the profile failed to resolve", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("currentApiConfigName", "shared-profile") + await provider.contextProxy.setValue("apiProvider", providerIdentifiers.openrouter) + await provider.contextProxy.setValue("viewStates", { + "unresolved-view": { + claimToken: "claim-x", + currentApiConfigName: "deleted-profile", + updatedAt: Date.now(), + }, + }) + + await provider["setViewStateId"]("unresolved-view", "claim-x") + + expect(provider["viewLocalState"].currentApiConfigName).toBe("deleted-profile") + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + // Defined behavior for a half-loaded view: the shared name and settings stay a pair. + expect(provider.getValues().currentApiConfigName).toBe("shared-profile") + expect(provider.getValues().apiProvider).toBe(providerIdentifiers.openrouter) + + await provider.dispose() + }) + }) + + describe("provider profile activation rollback ownership", () => { + const activationFailure = (provider: ClineProvider, otherViewSettings?: Record) => { + Object.assign(provider["providerSettingsManager"], { + saveConfig: vi.fn().mockResolvedValue("new-profile-id"), + listConfig: vi.fn().mockResolvedValue([]), + setModeConfig: vi.fn().mockRejectedValue(new Error("mode binding write failed")), + getModeConfigId: vi + .fn() + .mockResolvedValueOnce("previous-profile-id") + .mockImplementation(async () => { + // A second view activates its own profile between this group and the rollback. + if (otherViewSettings) { + await provider.contextProxy.setProviderSettings(otherViewSettings as never) + } + + return otherViewSettings ? undefined : "new-profile-id" + }), + }) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + } + + it("restores the shared writes when the per-view mirror rejects", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.openrouter } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + provider["viewLocalState"].currentApiConfigName = "prior-profile" + vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({ + ...nextEntry, + apiKey: "next-key", + } as never) + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([priorEntry, nextEntry]) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined) + // @ts-ignore - fail only the per-view mirror, after the shared writes landed. + vi.spyOn(provider, "_saveViewLocalStateFromMutation").mockRejectedValue(new Error("mirror failed")) + + await expect(provider.activateProviderProfile({ name: "next-profile" })).rejects.toThrow("mirror failed") + + // The shared selection must not name a profile the view buffer does not follow. + expect(provider.contextProxy.getValue("currentApiConfigName")).toBe("prior-profile") + await provider.dispose() + }) + it("mirrors the activated profile into the view buffer and durable pin", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.openrouter } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + provider["viewLocalState"].currentApiConfigName = "prior-profile" + provider["viewLocalState"].apiConfiguration = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "prior-key", + } + vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({ + ...nextEntry, + apiKey: "next-key", + } as never) + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([priorEntry, nextEntry]) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined) + + await provider.activateProviderProfile({ name: "next-profile" }) + + // getValues() overlays the view buffer over the shared store: without the mirror the + // previous profile's name and settings shadow the selection that was just activated. + expect(provider.getValues().currentApiConfigName).toBe("next-profile") + expect(provider["viewLocalState"].apiConfiguration?.apiKey).toBe("next-key") + await provider.dispose() + }) + it("names the newly activated profile in the view buffer and its durable pin on an activating save", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.openrouter } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + await provider["setViewStateId"]("stable-sidebar-view") + // The scenario the review row describes: this view is pinned to profile A in its buffer and + // in the durable record, and it is holding A settings. saveViewState writes both halves, so + // the pin is a real pin and not a primed field. + await provider.saveViewState("currentApiConfigName", "prior-profile") + provider["viewLocalState"].apiConfiguration = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "prior-key", + } + vi.spyOn(provider.providerSettingsManager, "saveConfig").mockResolvedValue("next-id") + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([priorEntry, nextEntry]) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined) + + // Record every write this view makes to its own buffer and durable pin, in order, by + // wrapping the private mutation entry point. Bracket access reaches the private member + // without an assertion, and the wrapper delegates so behaviour is unchanged. + const viewWrites: (Partial & Partial)[] = [] + const mirrorWrite = provider["_saveViewLocalStateFromMutation"] + provider["_saveViewLocalStateFromMutation"] = async (values) => { + // The buffer is mutated in place, so record a snapshot of the payload rather + // than the object the mutation path hands over. + viewWrites.push({ ...values }) + await mirrorWrite.call(provider, values) + } + + // A save that activates: the shared stores and this view mirror must agree on B. + await provider["persistProviderProfile"]( + "next-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ) + + // getValues() overlays this view buffer over the shared store. The reviewer fix is that one + // mutation carries the name together with the settings, so this view is never read in a + // state where a new name sits on the previous profile settings. Pin that, then both halves + // of the end state. + expect(viewWrites.some((values) => "currentApiConfigName" in values && "apiConfiguration" in values)).toBe( + true, + ) + // The ordering this branch keeps on top of their group: the self refresh that follows the + // group still runs last, after every activation write landed. + expect(viewWrites.at(-1)?.apiConfiguration).toEqual({ + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "new-model", + }) + // A mirror that carried only apiConfiguration would leave the buffered name A shadowing the + // profile that was just activated, which is what the webview would then be shown. + expect(provider.getValues().currentApiConfigName).toBe("next-profile") + expect(provider["viewLocalState"].apiConfiguration?.openRouterModelId).toBe("new-model") + // The durable pin has to follow too: a reload restores the entry below, and an entry that + // still names A loses the activation the user just performed. + expect(provider.contextProxy.getValue("viewStates")).toMatchObject({ + "stable-sidebar-view": { currentApiConfigName: "next-profile" }, + }) + await provider.dispose() + }) + // The activation group's shared name write must not carry this view's per-view mirror with it. + it("restores this activation's mode binding when this view's durable write rejects", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.openrouter } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + provider["viewLocalState"].currentApiConfigName = "prior-profile" + provider["viewLocalState"].apiConfiguration = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "prior-key", + } + vi.spyOn(provider.providerSettingsManager, "saveConfig").mockResolvedValue("next-id") + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([priorEntry, nextEntry]) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined) + // The mode starts bound to the profile this activation replaces; by the time the rollback runs + // it points at this activation's profile, which is what the restore must recognise as its own. + vi.spyOn(provider.providerSettingsManager, "getModeConfigId") + .mockResolvedValueOnce("prior-id") + .mockResolvedValue("next-id") + const setModeConfig = vi + .spyOn(provider.providerSettingsManager, "setModeConfig") + .mockResolvedValue(undefined) + const logSpy = vi.spyOn(provider, "log") + // Only this view's durable write fails; every shared store lands. The seam stays open, so + // the rollback's own durable write fails with the same reason and the save reports both. + const contextProxy = provider.contextProxy + const updateGlobalState = contextProxy.updateGlobalState.bind(contextProxy) + vi.spyOn(contextProxy, "updateGlobalState").mockImplementation(async (key, value) => { + if (key === "viewStates") { + throw new Error("viewStates write failed") + } + return updateGlobalState(key, value) + }) + const setValue = contextProxy.setValue.bind(contextProxy) + vi.spyOn(contextProxy, "setValue").mockImplementation(async (key, value) => { + if (key === "viewStates") { + throw new Error("viewStates write failed") + } + return setValue(key, value) + }) + + await expect( + provider["persistProviderProfile"]( + "next-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ), + ).rejects.toThrow("viewStates write failed") + + // The failed per-view write used to restore the shared name through this.setValue, and that + // restore advanced the shared-selection generation this activation was tracked against. The + // mode-binding restore is gated on that generation, so the rollback read its own undo as + // another view's write and left the mode bound to the profile whose activation failed. + expect(setModeConfig).toHaveBeenNthCalledWith(1, "code", "next-id") + expect(setModeConfig).toHaveBeenNthCalledWith(2, "code", "prior-id") + expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("Skipping the mode-binding restore")) + expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("Skipping the shared-selection restore")) + expect(provider.getValues().currentApiConfigName).toBe("prior-profile") + await provider.dispose() + }) + + // A rollback must restore the settings the buffer held, not only the name it held. + it("restores this view's buffered settings when an activating save rolls back", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.openrouter } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + provider["viewLocalState"].currentApiConfigName = "prior-profile" + provider["viewLocalState"].apiConfiguration = { + apiProvider: providerIdentifiers.anthropic, + apiKey: "prior-key", + } + vi.spyOn(provider.providerSettingsManager, "saveConfig").mockResolvedValue("next-id") + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([priorEntry, nextEntry]) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "getCurrentTask").mockReturnValue(undefined) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("prior-id") + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + // The mirror mutation lands; the shared provider-settings write is what rejects. + vi.spyOn(provider.contextProxy, "setProviderSettings") + .mockRejectedValueOnce(new Error("provider settings write failed")) + .mockResolvedValue(undefined) + + await expect( + provider["persistProviderProfile"]( + "next-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ), + ).rejects.toThrow("provider settings write failed") + + // Restoring only the name paired the restored profile with the settings of the profile whose + // activation failed: getValues() overlays the buffer over the shared store, so the view + // served the previous name together with the new provider and model. + expect(provider["viewLocalState"].currentApiConfigName).toBe("prior-profile") + expect(provider["viewLocalState"].apiConfiguration?.apiKey).toBe("prior-key") + expect(provider.getValues().currentApiConfigName).toBe("prior-profile") + expect(provider.getValues().apiProvider).toBe(providerIdentifiers.anthropic) + expect(provider["viewLocalState"].apiConfiguration?.apiProvider).toBe(providerIdentifiers.anthropic) + await provider.dispose() + }) + + it("should undo this activation's mode binding and settings while it still owns them", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setProviderSettings({ apiProvider: providerIdentifiers.anthropic }) + const setModeConfig = vi + .fn() + .mockRejectedValueOnce(new Error("mode binding write failed")) + .mockResolvedValue(undefined) + Object.assign(provider["providerSettingsManager"], { + saveConfig: vi.fn().mockResolvedValue("new-profile-id"), + listConfig: vi.fn().mockResolvedValue([]), + setModeConfig, + // First read: the binding this activation captured. Second read: still ours. + getModeConfigId: vi + .fn() + .mockResolvedValueOnce("previous-profile-id") + .mockResolvedValue("new-profile-id"), + }) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + + await expect( + provider["persistProviderProfile"]( + "test-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ), + ).rejects.toThrow("mode binding write failed") + + // The group write, then the rollback of the binding this activation still owns. + expect(setModeConfig).toHaveBeenNthCalledWith(1, "code", "new-profile-id") + expect(setModeConfig).toHaveBeenNthCalledWith(2, "code", "previous-profile-id") + expect(provider.contextProxy.getValue("apiProvider")).toBe(providerIdentifiers.anthropic) + expect(provider.contextProxy.getValue("openRouterModelId")).toBeUndefined() + + await provider.dispose() + }) + + it("should leave a newer view's mode binding and provider settings alone", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setProviderSettings({ apiProvider: providerIdentifiers.anthropic }) + activationFailure(provider, { + apiProvider: providerIdentifiers.requesty, + requestyModelId: "other-view-model", + }) + const logSpy = vi.spyOn(provider, "log") + + await expect( + provider["persistProviderProfile"]( + "test-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ), + ).rejects.toThrow("mode binding write failed") + + expect(provider["providerSettingsManager"].setModeConfig).toHaveBeenCalledTimes(1) + expect(provider.contextProxy.getValue("apiProvider")).toBe(providerIdentifiers.requesty) + expect(provider.contextProxy.getValue("requestyModelId")).toBe("other-view-model") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Skipping the mode-binding restore")) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Skipping the shared-settings restore")) + + await provider.dispose() + }) + it("names the store whose undo failed when a rollback write also rejects", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const priorEntry = { name: "prior-profile", id: "prior-id", apiProvider: providerIdentifiers.anthropic } + const nextEntry = { name: "next-profile", id: "next-id", apiProvider: providerIdentifiers.anthropic } + await provider.contextProxy.setValue("listApiConfigMeta", [priorEntry, nextEntry]) + await provider.contextProxy.setValue("currentApiConfigName", "prior-profile") + await provider.contextProxy.setProviderSettings({ apiProvider: providerIdentifiers.anthropic }) + // The activation group write rejects, and so does the undo of the same store. + const setModeConfig = vi + .fn() + .mockRejectedValueOnce(new Error("mode binding write failed")) + .mockRejectedValueOnce(new Error("mode binding undo failed")) + .mockResolvedValue(undefined) + Object.assign(provider["providerSettingsManager"], { + saveConfig: vi.fn().mockResolvedValue("new-profile-id"), + listConfig: vi.fn().mockResolvedValue([]), + setModeConfig, + // First read: the binding this activation captured. Second read: still ours. + getModeConfigId: vi + .fn() + .mockResolvedValueOnce("previous-profile-id") + .mockResolvedValue("new-profile-id"), + }) + vi.spyOn(provider, "getState").mockResolvedValue({ mode: "code" } as never) + const logSpy = vi.spyOn(provider, "log") + let failure: unknown + try { + await provider["persistProviderProfile"]( + "test-profile", + { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "new-model" }, + true, + new AbortController().signal, + ) + throw new Error("the activation was expected to fail") + } catch (error) { + failure = error + } + const rollbackError = failure as Error + // The original failure stays at the head of the message and stays the cause, + // so a caller can branch on either. The failed undo is named beside it. + expect(rollbackError.message).toContain("mode binding write failed") + expect(rollbackError.message).toContain( + "activation rollback also failed: mode binding: mode binding undo failed", + ) + expect((rollbackError.cause as Error).message).toBe("mode binding write failed") + // Only the undo that failed is reported: the store is named, not the whole group. + expect(rollbackError.message).not.toContain("profile list:") + expect(rollbackError.message).not.toContain("shared selection:") + // Every other store this activation still owned was undone anyway. + expect(setModeConfig).toHaveBeenNthCalledWith(2, "code", "previous-profile-id") + expect(provider.contextProxy.getValue("listApiConfigMeta")).toEqual([priorEntry, nextEntry]) + expect(provider.contextProxy.getValue("currentApiConfigName")).toBe("prior-profile") + expect(provider.contextProxy.getValue("apiProvider")).toBe(providerIdentifiers.anthropic) + // Nothing was skipped for ownership: the skips log, a failed undo throws. + expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("Skipping the")) + await provider.dispose() + }) + }) + + it("should merge concurrent persisted updates from separate provider instances without lost viewStates", async () => { + const provider1 = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + + new WebviewFocusTracker(), + ) + const provider2 = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider1["setViewStateId"]("stable-sidebar-view") + await provider2["setViewStateId"]("stable-editor-view") + + await Promise.all([ + provider1.saveViewState("mode", "architect"), + provider2.saveViewState("currentApiConfigName", "editor-profile"), + ]) + + expect(mockContext.globalState.get("viewStates")).toMatchObject({ + "stable-sidebar-view": { mode: "architect" }, + "stable-editor-view": { currentApiConfigName: "editor-profile" }, + }) + + await provider1.dispose() + await provider2.dispose() + }) + + describe("view state persistence edge cases", () => { + it("should load persisted mode, profile name and resolved profile into viewLocalState", async () => { + const writer = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await writer["setViewStateId"]("shared-view") + await writer.saveViewState("mode", "architect") + await writer.saveViewState("currentApiConfigName", "my-profile") + + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + const getProfileSpy = vi.fn().mockResolvedValue({ + name: "my-profile", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "model-x", + }) + // @ts-ignore - Replace providerSettingsManager with a test double for the profile lookup. + provider.providerSettingsManager = { getProfile: getProfileSpy } + // The ownership check requires the writer to release the id before the + // next view adopts it; the durable entry it wrote stays in storage. + await writer.dispose() + + await provider.contextProxy.setValue( + "viewStates", + mockContext.globalState.get("viewStates"), + ) + // The host-minted claim the writer received is what lets this view re-adopt its entry. + await provider["setViewStateId"]("shared-view", persistedViewStateClaim(mockContext, "shared-view")) + expect(provider["viewLocalState"]).toEqual({ + mode: "architect", + currentApiConfigName: "my-profile", + apiConfiguration: { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "model-x" }, + }) + expect(getProfileSpy).toHaveBeenCalledWith({ name: "my-profile" }) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Loaded state for viewId")) + await provider.dispose() + }) + + it("should apply setValue mutations to global state and keep or clear the right buffer fields", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const apiConfiguration = { apiProvider: providerIdentifiers.openrouter } + await provider.saveViewState("mode", "architect") + await provider.saveViewState("currentApiConfigName", "my-profile") + await provider.saveViewState("apiConfiguration", apiConfiguration) + // A mutation of an unrelated key reaches global state without dropping buffered fields. + await provider.setValue("writeDelayMs", 500) + expect(mockContext.globalState.get("writeDelayMs")).toBe(500) + expect(provider.getValues().writeDelayMs).toBe(500) + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile") + expect(provider["viewLocalState"].apiConfiguration).toBe(apiConfiguration) + await provider.setValue("mode", undefined) + expect(provider["viewLocalState"]).not.toHaveProperty("mode") + await provider.setValue("currentApiConfigName", undefined) + expect(provider["viewLocalState"]).not.toHaveProperty("currentApiConfigName") + await provider.dispose() + }) + + it("should remove the buffered apiConfiguration when it is cleared", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const save = provider.saveViewState.bind(provider) as (key: string, value: unknown) => Promise + await provider.saveViewState("apiConfiguration", { apiProvider: providerIdentifiers.openrouter }) + await provider.saveViewState("apiConfiguration", undefined) + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + await provider.saveViewState("apiConfiguration", { apiProvider: providerIdentifiers.openrouter }) + await save("apiConfiguration", null) + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + await provider.dispose() + }) + + it("should roll back the shared write when setValue per-view persistence fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "architect") + const originalUpdate = mockContext.globalState.update + mockContext.globalState.update = vi.fn().mockImplementation((key: string) => { + if (key === "viewStates") { + return Promise.reject(new Error("storage unavailable")) + } + return Promise.resolve() + }) + + await expect(provider.setValue("mode", "code")).rejects.toThrow("storage unavailable") + // The shared store must not move ahead of the per-view record that failed. + expect(provider.getValue("mode")).toBe("architect") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + }) }) describe("webviewMessageHandler no-floating-promises coverage", () => {