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..29a254be1b 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -112,6 +112,7 @@ export interface ExtensionMessage { | "originalContent" | "rooHistoryImportProgress" | "themeFixtureProbeRequest" + | "viewStateClaim" | "diagnosticsRequest" text?: string /** For fileContent: { path, content, error? } */ @@ -171,6 +172,13 @@ export interface ExtensionMessage { // eslint-disable-next-line @typescript-eslint/no-explicit-any values?: Record requestId?: string + /** + * For `viewStateClaim`: the durable `viewStates` key the delivered claim governs. The + * message is only ever sent to the webview that key belongs to. + */ + viewStateId?: string + /** For `viewStateClaim`: the host-minted token the webview stores and echoes on launch. */ + viewStateClaim?: string themeFixture?: WebviewThemeFixture diagnostics?: WebviewDiagnosticsSnapshot promptText?: string @@ -712,6 +720,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..971d045e0e 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -67,14 +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 + // Raised for the whole duration of a reset and cleared in a finally, so a reset that fails + // mid-way cannot leave the store refusing every later secret write; see resetAllState. + private resetting = false constructor(context: vscode.ExtensionContext) { this.originalContext = context @@ -413,6 +421,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 +439,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 +458,13 @@ export class ContextProxy { } storeSecret(key: SecretStateKey, value?: string) { + // 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. + if (this.resetting) { + 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) @@ -716,17 +738,27 @@ 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 { + // 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) + } - 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() + } 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..c1bca05e6d 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -121,6 +121,186 @@ 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() + rejectPending(new Error("disk full")) + + await expect(pending).rejects.toThrow("disk full") + // The reset cleared the cache: restoring the pre-reset value would resurrect state + // that storage no longer holds. + expect(proxy.getGlobalState("apiProvider")).toBeUndefined() + }) + // This branch carried no write guard at all before these two cases: resetAllState cleared the + // caches and deleted storage with nothing telling storeSecret that a reset was running, so a + // save issued mid-reset could land after the deletes and resurrect a value the user had just + // erased. The refusal is the new observable here rather than an existing one, so it gets its + // own case instead of being a phase of the leak case. + it("refuses a secret write issued while a reset is still inside its delete pass", async () => { + const erases: { reject: (error: Error) => void }[] = [] + mockSecrets.delete.mockImplementation((key: string) => { + if (key === "apiKey") { + return new Promise((_, reject) => { + erases.push({ reject }) + }) + } + return Promise.resolve() + }) + + const resetting = proxy.resetAllState() + // The reset yields before it reaches its delete pass, so let it get there before asking the + // store to write. + await new Promise((resolve) => { + setTimeout(resolve, 10) + }) + expect(erases.length).toBe(1) + + const refused = await proxy.storeSecret("apiKey", "saved-mid-reset").then( + () => undefined, + (error: unknown) => error, + ) + expect(refused).toBeInstanceOf(SettingsWriteError) + expect(String((refused as Error).message)).toContain("secret writes are refused while state is being reset") + // Refused rather than deferred: a deferred write would re-store the value this reset is + // in the middle of erasing. + // Refused rather than deferred, and not silently written either: the cache the reset already + // cleared stays empty and no store call was made for the key. + expect(proxy.getSecret("apiKey")).toBeUndefined() + expect(mockSecrets.store.mock.calls.filter((call: readonly unknown[]) => call[0] === "apiKey").length).toBe( + 0, + ) + + erases.forEach((erase) => { + erase.reject(new Error("keychain locked")) + }) + await expect(resetting).rejects.toThrow("keychain locked") + }) + it("accepts a secret write again after a reset whose delete pass rejected", 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) + }) + + await expect(proxy.resetAllState()).rejects.toThrow("keychain locked") + + // The half that bites: the delete pass is a single Promise.all, so one keychain rejection + // rejects resetAllState itself. Unless the guard is lowered on that path too, every later + // save is refused for the life of the instance while resetState() has already aborted on the + // same error, so the user cannot get a key back without restarting the extension host. + // Measured as a save that resolves and reads back through storage and cache, not as an + // absence of errors. + await expect(proxy.storeSecret("apiKey", "written-after-failed-reset")).resolves.toBeUndefined() + expect(await mockSecrets.get("apiKey")).toBe("written-after-failed-reset") + expect(proxy.getSecret("apiKey")).toBe("written-after-failed-reset") + }) + + it("should restore the last persisted value when two overlapping writes both reject", async () => { + // The fake now models storage: an update commits its value and a read sees what storage holds, + // which is what the cache correction on a rejected write reads back. A write that rejects + // commits nothing, so storage still holds the last value it accepted. + const storage = new Map() + mockGlobalState.get.mockImplementation((key: string) => storage.get(key)) + mockGlobalState.update.mockImplementation(async (key: string, value: unknown) => { + storage.set(key, value) + }) + await proxy.updateGlobalState("apiProvider", "anthropic") + let rejectFirst: (error: Error) => void = () => {} + let rejectSecond: (error: Error) => void = () => {} + mockGlobalState.update + .mockReturnValueOnce( + new Promise((_, reject) => { + rejectFirst = reject + }), + ) + .mockReturnValueOnce( + new Promise((_, reject) => { + rejectSecond = reject + }), + ) + + const firstWrite = proxy.updateGlobalState("apiProvider", "openrouter") + const secondWrite = proxy.updateGlobalState("apiProvider", "deepseek") + rejectSecond(new Error("second failed")) + await expect(secondWrite).rejects.toThrow("second failed") + rejectFirst(new Error("first failed")) + await expect(firstWrite).rejects.toThrow("first failed") + + // Neither attempt reached storage: the cache must hold the last value storage accepted, + // not the first attempt's unpersisted one. + expect(proxy.getGlobalState("apiProvider")).toBe("anthropic") + }) + + it("should restore the loaded value when overlapping writes reject with no prior write", async () => { + // Cold start: storage holds the value but nothing has been written through this + // proxy yet, so the persisted-value map has no entry for the key. + mockGlobalState.get.mockImplementation((key: string) => (key === "apiProvider" ? "openai" : undefined)) + const coldProxy = new ContextProxy(mockContext) + await coldProxy.initialize() + expect(coldProxy.getGlobalState("apiProvider")).toBe("openai") + + mockGlobalState.update.mockReset() + let rejectFirst: (error: Error) => void = () => {} + let rejectSecond: (error: Error) => void = () => {} + mockGlobalState.update + .mockReturnValueOnce( + new Promise((_, reject) => { + rejectFirst = reject + }), + ) + .mockReturnValueOnce( + new Promise((_, reject) => { + rejectSecond = reject + }), + ) + + const firstWrite = coldProxy.updateGlobalState("apiProvider", "openrouter") + const secondWrite = coldProxy.updateGlobalState("apiProvider", "deepseek") + rejectSecond(new Error("second failed")) + await expect(secondWrite).rejects.toThrow("second failed") + rejectFirst(new Error("first failed")) + await expect(firstWrite).rejects.toThrow("first failed") + + // The second write's `previousCachedValue` is the first write's pending value, so the + // fallback branch would leave a value that storage never accepted. + expect(coldProxy.getGlobalState("apiProvider")).toBe("openai") + }) + it("should bypass cache for pass-through state keys", async () => { // Setup mock return value mockGlobalState.get.mockReturnValue("pass-through-value") @@ -1170,4 +1350,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..3b93b7cd22 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,6 +973,11 @@ 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", ) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 6c66655325..b8c5cb3327 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -331,6 +331,51 @@ 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) + // Per-view selection state is machine-local: importing settings must not + // apply another machine's view pins, while other settings round-trip. + 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..29c9497552 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -98,6 +98,14 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): { for (const [key, rawValue] of Object.entries(rawGlobalSettings)) { const path = `globalSettings.${key}` + + // Per-view selection state is machine-local: it round-trips through the + // normal runtime and pruning paths, but importing it would pin selections + // from another machine's views on this one. + 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 6cec3bdbe7..ca9d5daffb 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" @@ -130,9 +132,9 @@ import { t } from "../../i18n" import { buildApiHandler } from "../../api" import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../api/providers/fetchers/lmstudio" -import { ContextProxy } from "../config/ContextProxy" +import { ContextProxy, isPassThroughStateKey } from "../config/ContextProxy" import { withoutInvalidDisplaySettings } from "../config/displaySettings" -import { ProviderSettingsManager } from "../config/ProviderSettingsManager" +import { ProviderSettingsManager, ProviderSettingsNotFoundError } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" import { PendingActionSettlementError, Task } from "../task/Task" @@ -160,6 +162,21 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" +type PersistedViewState = NonNullable[string] + +/** + * Values that can be held in a view-local state buffer (in-memory) and, for the + * non-secret subset, persisted durably per stable view id. + */ +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 +265,10 @@ 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() private webviewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< string, @@ -415,6 +436,42 @@ 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 = {} + + // Bumped whenever the buffer is cleared (reset/import), so an in-flight load can tell + // that its captured state was invalidated while it awaited the profile lookup. + private viewLocalStateGeneration = 0 + + /** + * 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 = "nov-2026-v3.96.0-settings-costs-reliability" // v3.96.0 chat settings, provider costs, and reliability @@ -430,6 +487,11 @@ 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 + ClineProvider.activeInstances.add(this) this.currentWorkspacePath = getWorkspacePath() this.pendingEditOperations = new PendingEditOperationStore( ClineProvider.PENDING_OPERATION_TIMEOUT_MS, @@ -463,6 +525,9 @@ export class ClineProvider await this.postStateToWebviewWithoutClineMessages() }) + // Load initial state from global state into viewLocalState buffer after dependencies used by getState are ready. + void this.loadViewState() + // Initialize MCP Hub through the singleton manager McpServerManager.getInstance(this.context, this) .then((hub) => { @@ -599,6 +664,450 @@ 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 { + // 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 () => { + 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)) + // 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) + } + } + + /** + * 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)) + 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. + */ + private prunePersistedViewStates(states: Record): Record { + // Entries of live views must never age out: a view that keeps its mode and profile does + // not refresh updatedAt. The cap applies to closed-view entries only. + const liveViewStateIds = new Set( + Array.from(ClineProvider.activeInstances, (instance) => instance.viewStateId).filter( + (viewStateId): viewStateId is string => Boolean(viewStateId), + ), + ) + const liveEntries = Object.entries(states).filter(([viewStateId]) => liveViewStateIds.has(viewStateId)) + const closedEntries = Object.entries(states) + .filter(([viewStateId]) => !liveViewStateIds.has(viewStateId)) + .sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)) + .slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES) + + return Object.fromEntries([...liveEntries, ...closedEntries]) + } + + /** 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)) + + // 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 { + const loadedGeneration = this.viewLocalStateGeneration + // 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) { + try { + const { name: _name, ...apiConfiguration } = await this.providerSettingsManager.getProfile({ + name: persisted.currentApiConfigName, + }) + // Pin the name only together with the configuration it resolves to: a name + // without its overlay would pair with whatever shared settings sit under it. + loadedState.currentApiConfigName = 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 + } + } + + // The profile name and the settings it resolved to are one pair: the configuration loaded + // above belongs to the profile the lookup asked for, so it must not stay parked under a + // name this view changed while the lookup was in flight. A name this view changed on its + // own pins nothing - reads fall back to the shared settings, exactly as every other + // name-only write behaves. + if ( + loadedState.apiConfiguration !== undefined && + Object.is(preLoadBuffer.apiConfiguration, postLoadBuffer.apiConfiguration) && + mergedState.currentApiConfigName !== loadedState.currentApiConfigName + ) { + delete mergedState.apiConfiguration + } + + // A reset/import may have cleared the buffer while the lookup was in flight: + // republishing here would restore the pre-reset selection over the cleared state. + if (this.viewLocalStateGeneration !== loadedGeneration) { + this.log(`[loadViewState] Discarding state loaded before a reset for viewId ${this.viewId}`) + return + } + + 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 */ @@ -944,7 +1453,6 @@ export class ClineProvider ) } } - async dispose() { if (this._disposed) { return @@ -1485,7 +1993,10 @@ export class ClineProvider historyItem.mode = defaultModeSlug } - await this.updateGlobalState("mode", historyItem.mode) + // Persist the restored mode through this view's per-view pin rather than the + // shared global: a global write would leak the restored mode into other views + // in parallel mode, and a buffer-only write would be lost after a reload. + await this.saveViewState("mode", historyItem.mode) // Load the saved API config for the restored mode if it exists. // Skip mode-based profile activation if historyItem.apiConfigName exists, @@ -1513,6 +2024,15 @@ export class ClineProvider const hasActualSettings = !!fullProfile.apiProvider if (hasActualSettings) { + // The restored mode lives only in this view's buffer, so the shared + // mode slot must not be written: activateProviderProfile reads mode + // through getState(), which does not see the view-local overlay, and + // would bind this profile to whichever mode the shared store still + // reports. The profile was read from the mode slot, so re-persisting + // it here is redundant as well as wrong. + // The task id guard is upstream's: restoring a history item must not rebuild + // or sticky-write a task that another view swapped in while this restoration + // was still in flight. await this.activateProviderProfile( { name: profile.name }, { persistModeConfig: false, expectedCurrentTaskId: historyItem.id }, @@ -1722,18 +2242,43 @@ export class ClineProvider return } + const webview = this.view?.webview + if (!webview) { + return + } + + // Dispatch without awaiting the renderer ack: VS Code settles postMessage only when the + // webview page acknowledges the message, and a page reload or view dispose in flight + // orphans that promise forever. Awaiting it could wedge every caller on the task critical + // path (e.g. the trailing postStateToWebview in handleModeSwitchUnlocked gates the next + // turn after a mode switch). Message ordering is enforced by the message seq, not the ack. + // Promise.resolve() normalizes non-promise returns (e.g. test doubles) before the catch. + // A disposed webview can also throw synchronously from postMessage itself, before + // any promise exists to attach the catch to: route that path through the same + // log so fire-and-forget callers never surface an unhandled rejection. + const logDropped = (error: unknown) => { + // Swallow: postMessage rejects (or throws) when the webview is disposed. + // Log the dropped message type so a wedged webview channel is diagnosable + // instead of silently losing state updates. + this.log( + `[postMessageToWebview] dropped message type=${message.type}: ${error instanceof Error ? error.message : String(error)}`, + ) + } try { - const delivered = await this.view?.webview.postMessage(omitOriginalContentFromExtensionMessage(message)) - this.diagnosticsRecorder.record({ - boundary: "webview-out", - phase: delivered ? "success" : "failure", - type: message.type, - action: message.action, - elapsedMs: Date.now() - startedAt, - stateSequence: message.type === "state" ? message.state?.clineMessagesSeq : undefined, - taskId, - }) - } catch { + void Promise.resolve(webview.postMessage(omitOriginalContentFromExtensionMessage(message))) + .then((delivered) => { + this.diagnosticsRecorder.record({ + boundary: "webview-out", + phase: delivered ? "success" : "failure", + type: message.type, + action: message.action, + elapsedMs: Date.now() - startedAt, + stateSequence: message.type === "state" ? message.state?.clineMessagesSeq : undefined, + taskId, + }) + }) + .catch(logDropped) + } catch (error) { this.diagnosticsRecorder.record({ boundary: "webview-out", phase: "failure", @@ -1742,7 +2287,7 @@ export class ClineProvider elapsedMs: Date.now() - startedAt, taskId, }) - // View disposed, drop message silently + logDropped(error) } } @@ -2058,7 +2603,6 @@ export class ClineProvider if (task) { TelemetryService.instance.captureModeSwitch(task.taskId, newMode) - task.emit(RooCodeEventName.TaskModeSwitched, task.taskId, newMode) try { // Update the task history with the new mode first. @@ -2068,6 +2612,8 @@ export class ClineProvider await this.updateTaskHistory({ ...taskHistoryItem, mode: newMode }) } + task.emit(RooCodeEventName.TaskModeSwitched, task.taskId, newMode) + // Only update the task's mode after successful persistence. ;(task as any)._taskMode = newMode } catch (error) { @@ -2082,7 +2628,20 @@ export class ClineProvider } } - await this.updateGlobalState("mode", newMode) + // setValue (not the deprecated updateGlobalState) so the in-memory viewLocalState + // buffer stays in sync with the durable global write: getValues() merges + // viewLocalState on top of the ContextProxy values, so an unsynced stale + // restored mode would otherwise shadow the fresh switch for consumers. + // setValue already restores the shared value when the per-view write fails, and that + // restore is value-guarded: an unconditional rollback here would overwrite a newer mode. + try { + await this.setValue("mode", newMode) + } catch (error) { + this.log( + `[handleModeSwitch] Failed to persist mode "${newMode}": ${error instanceof Error ? error.message : String(error)}`, + ) + throw error + } this.emit(RooCodeEventName.ModeChanged, newMode) @@ -2095,6 +2654,9 @@ export class ClineProvider return } + // A provider-profile mutation timeout must stop this path instead of letting it + // persist a profile the running task no longer agrees with: guard before the reads + // and again after them, because an abort can land while they are in flight. if (signal?.aborted) return // Load the saved API config for the new mode if it exists. @@ -2356,9 +2918,12 @@ export class ClineProvider // `allSettled` so a caller that rolls back after a failure never races a write still in flight. const activationResults = await Promise.allSettled([ this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()), - this.updateGlobalState("currentApiConfigName", name), + // Route through setValue so the buffer tracks the activated profile. + this.setValue("currentApiConfigName", name), this.providerSettingsManager.setModeConfig(mode, id), this.contextProxy.setProviderSettings(providerSettings), + // setProviderSettings bypasses the buffer path: refresh this view's overlay. + this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }), ]) const failedActivation = activationResults.find( (result): result is PromiseRejectedResult => result.status === "rejected", @@ -2368,6 +2933,10 @@ export class ClineProvider if (signal.aborted) return id + // Refresh every other live view pinned to this profile, and rebuild the tasks that + // follow it, so no sibling keeps serving the pre-save settings under the same name. + 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 +2945,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 this view when it + // pins the saved profile; its siblings are refreshed by the helper below. + 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) and rebuild the tasks that follow the + // profile. This branch had no sibling refresh at all before this change. + await this.refreshSiblingsAfterProfileSave(name, providerSettings) } if (signal.aborted) return id @@ -2384,6 +2967,33 @@ export class ClineProvider 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() let profileToActivate: string | undefined = globalSettings.currentApiConfigName @@ -2396,15 +3006,163 @@ export class ClineProvider throw new Error("You cannot delete the last profile") } + // Remove the profile from the settings store (context.secrets) so it cannot be + // resurrected by a later listApiConfigMeta sync. A not-found rejection means + // the secret was already gone (e.g. pruned by an earlier run): branch on the + // typed ProviderSettingsNotFoundError so the stale list entry below is still + // pruned as an idempotent success, while any other failure (e.g. refusing to + // delete the last remaining configuration) propagates. Matching message text + // instead would let a profile whose name contains "not found" swallow an + // unrelated failure. + try { + await this.providerSettingsManager.deleteConfig(profileToDelete.name) + } catch (error) { + if (!(error instanceof ProviderSettingsNotFoundError)) { + throw error + } + this.log( + `deleteProviderProfile: settings for '${profileToDelete.name}' were not found; pruning the stale list entry only`, + ) + } + const entries = this.getProviderProfileEntries().filter(({ name }) => name !== profileToDelete.name) - await this.contextProxy.setValues({ - ...globalSettings, - currentApiConfigName: profileToActivate, - listApiConfigMeta: entries, - }) + // Write only the profile list back: replaying the full settings snapshot + // captured above would also rewrite unrelated keys (including viewStates, + // which ClineProvider mutates directly in storage for concurrent views) + // with this view's stale cached copy. + await this.contextProxy.setValue("listApiConfigMeta", entries) + + // Resolve the surviving profile's settings so this view and any other + // live view still pinned to the deleted profile can be re-pinned with + // a matching configuration. + let survivingSettings: ProviderSettings | undefined + try { + const { name: _survivingName, ...settings } = await this.providerSettingsManager.getProfile({ + name: profileToActivate, + }) + survivingSettings = settings as ProviderSettings + } catch (error) { + this.log( + `[deleteProviderProfile] Unable to resolve API profile '${profileToActivate}': ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + + // Capture this view's pin before any rewrite: a view pinned to the + // deleted profile while the global selection points elsewhere must still be + // reconfigured, or getState() would keep the deleted profile's settings under + // the surviving profile's name. + const viewWasPinnedToDeleted = this.viewLocalState.currentApiConfigName === profileToDelete.name + const deletedWasGlobal = profileToDelete.name === globalSettings.currentApiConfigName + + // deleteConfig removed the profile but leaves the settings manager's stored + // currentApiConfigName naming it, so a later export would write a selection the + // profile list no longer contains (and import would silently pick another profile). + // Point the manager at the survivor too. + let managerNameUpdateFailure: Error | undefined + if (deletedWasGlobal) { + try { + await this.providerSettingsManager.activateProfile({ name: profileToActivate }) + } catch (firstError) { + try { + // Retry once: a stale manager name would be exported as a selection the + // profile list no longer contains. + await this.providerSettingsManager.activateProfile({ name: profileToActivate }) + } catch (retryError) { + this.log( + `[deleteProviderProfile] Failed to update the settings manager's current profile to '${profileToActivate}' after retry: ${ + retryError instanceof Error ? retryError.message : String(retryError) + } (first attempt: ${firstError instanceof Error ? firstError.message : String(firstError)})`, + ) + // The deletion has committed: finish the shared-state repair and post it, then + // surface the incomplete manager update to the caller. + managerNameUpdateFailure = new Error( + `Profile '${profileToDelete.name}' was deleted but the settings manager still names it as current.`, + { cause: retryError }, + ) + } + } + } + + if (viewWasPinnedToDeleted) { + // This view's pin now dangles: re-point it. setValue also persists the + // survivor to the shared store, which covers the deleted-was-global case + // for every other view as well as this one. + await this.setValue("currentApiConfigName", profileToActivate) + } else if (deletedWasGlobal) { + // The shared selection changed, but this view's own pin still names a + // surviving profile: update the shared store only, leaving the + // view-local pin untouched. + await this.contextProxy.setValue("currentApiConfigName", profileToActivate) + } + + if (deletedWasGlobal || viewWasPinnedToDeleted) { + if (survivingSettings) { + // The deleted profile was the active one (globally, or for this view), so + // the shared provider keys still carry its settings; replace them so + // getState() reports the surviving profile's configuration. + await this.contextProxy.setProviderSettings(survivingSettings) + + if (viewWasPinnedToDeleted) { + // This view's nested overlay (viewLocalState.apiConfiguration, seeded + // by loadViewState) still serves the deleted profile's configuration: + // replace it with the survivor's so the re-pointed pin serves matching + // settings. A view pinned to another profile keeps its own overlay. + await this._saveViewLocalStateFromMutation({ apiConfiguration: survivingSettings }) + } + + // A live task of this view that runs under the deleted profile keeps the handler + // built from its settings and the deleted sticky name until it is rebuilt - the same + // rule the activating-save branch applies. + if (this.getCurrentTask()?.taskApiConfigName === profileToDelete.name) { + this.updateTaskApiHandlerIfNeeded(survivingSettings, { forceRebuild: true }) + await this.persistStickyProviderProfileToCurrentTask(profileToActivate) + } + } else if (viewWasPinnedToDeleted) { + // The survivor's settings never resolved: this view's pin now names the survivor, so + // its overlay must stop serving the deleted profile's configuration and fall back to + // the shared settings instead. + await this._saveViewLocalStateFromMutation({ apiConfiguration: undefined }) + } + } + + // Re-pin other live views still buffered on the deleted profile: their buffer and + // durable viewStates entry would otherwise keep serving the deleted profile's name + // and configuration. + for (const instance of ClineProvider.getAllInstances()) { + if (instance === this || instance._disposed || instance.pinnedProfileName !== profileToDelete.name) { + continue + } + const values: Partial & Partial = { + currentApiConfigName: profileToActivate, + // Either the survivor's settings or an explicit clear: a view re-pointed away from the + // deleted profile must not keep serving that profile's configuration. + apiConfiguration: survivingSettings, + } + // Direct private access: compile-time safe across sibling instances. + await instance._saveViewLocalStateFromMutation(values) + // The re-pinned buffer alone does not reach a live task in that view: it still runs on a + // handler built from the deleted profile's settings and still names the deleted profile as + // its sticky profile. + if (survivingSettings && instance.getCurrentTask()?.taskApiConfigName === profileToDelete.name) { + instance.updateTaskApiHandlerIfNeeded(survivingSettings, { forceRebuild: true }) + await instance.persistStickyProviderProfileToCurrentTask(profileToActivate) + } + await instance.postStateToWebview().catch((postError: unknown) => { + this.log( + `[deleteProviderProfile] failed to post re-pinned state to a sibling view: ${ + postError instanceof Error ? postError.message : String(postError) + }`, + ) + }) + } await this.postStateToWebview() + if (managerNameUpdateFailure) { + throw managerNameUpdateFailure + } } private async persistStickyProviderProfileToCurrentTask( @@ -2474,9 +3232,21 @@ export class ClineProvider // See `upsertProviderProfile` for a description of what this is doing. await Promise.all([ this.contextProxy.setValue("listApiConfigMeta", await this.providerSettingsManager.listConfig()), - this.contextProxy.setValue("currentApiConfigName", name), + // Route through setValue so the in-memory viewLocalState buffer tracks the + // activated profile: a plain ContextProxy write would leave a stale loaded + // currentApiConfigName shadowing the new value in getValues(). + this.setValue("currentApiConfigName", name), this.contextProxy.setProviderSettings(providerSettings), + // setProviderSettings writes the shared store directly, bypassing the + // view-local mutation path: also refresh this view's buffer so a stale + // loaded apiConfiguration cannot keep shadowing the new settings in + // getState(). + this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }), ]) + + // Other live views may have buffered this profile's settings earlier; + // refresh them so their getState() cannot report the activated profile's + // name with stale settings. } const { mode } = await this.getState() @@ -2646,8 +3416,10 @@ export class ClineProvider // picks up the new token immediately for the current task. await this.upsertProviderProfile(entry.name, updated, true) } else { - // Non-active profiles just need the token saved to disk. - await this.providerSettingsManager.saveConfig(entry.name, updated) + // Non-active profiles need the token saved to disk and the views pinned + // to them refreshed: a direct saveConfig leaves a pinned view serving the + // old token until it reloads. + await this.upsertProviderProfile(entry.name, updated, false) } } } @@ -3559,7 +4331,10 @@ export class ClineProvider terminalZshP10k: stateValues.terminalZshP10k ?? false, terminalZdotdir: stateValues.terminalZdotdir ?? false, terminalProfile: stateValues.terminalProfile, - mode: stateValues.mode ?? defaultModeSlug, + // A per-view restored or edited mode lives in this view's buffer, and getState() is + // what task creation and getStateToPostToWebview read: the shared store only carries the + // mode the last committing view chose. + mode: this.viewLocalState.mode ?? stateValues.mode ?? defaultModeSlug, language: stateValues.language ?? formatLanguage(vscode.env.language), mcpEnabled: stateValues.mcpEnabled ?? true, mcpServers: this.mcpHub?.getAllServers() ?? [], @@ -3701,7 +4476,23 @@ export class ClineProvider } public async setValue(key: K, value: RooCodeSettings[K]) { + const previousSharedValues = { [key]: this.contextProxy.getValue(key) } as RooCodeSettings + const writeGeneration = this.contextProxy.getStateWriteGeneration(key as never) 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, + // This write is the generation after the one captured before it: only the write + // that is still the newest for the key may be rolled back. + { [key]: writeGeneration + 1 } as Record, + ) + throw error + } } public getValue(key: K) { @@ -3709,13 +4500,93 @@ export class ClineProvider } public getValues() { - return this.contextProxy.getValues() + return { ...this.contextProxy.getValues(), ...this.viewLocalState } } public async setValues(values: RooCodeSettings) { - await this.contextProxy.setValues( - withoutInvalidDisplaySettings(values, (key) => this.log(`[setValues] Ignored an invalid value for ${key}`)), + const sanitizedValues = withoutInvalidDisplaySettings(values, (key) => + this.log(`[setValues] Ignored an invalid value for ${key}`), ) + + if (sanitizedValues.mode !== undefined) { + // An unknown or non-string mode (e.g. from an API payload) must not be persisted: + // a new Task would read it from getState() and persist it into task history. + if ( + typeof sanitizedValues.mode !== "string" || + !getModeBySlug(sanitizedValues.mode, await this.customModesManager.getCustomModes()) + ) { + this.log(`[ClineProvider#setValues] Ignoring invalid mode "${String(sanitizedValues.mode)}"`) + delete sanitizedValues.mode + } + } + + const previousSharedValues = Object.fromEntries( + Object.keys(sanitizedValues).map((key) => [key, this.contextProxy.getValue(key as keyof RooCodeSettings)]), + ) as RooCodeSettings + const writeGenerations = Object.fromEntries( + Object.keys(sanitizedValues).map((key) => [ + key, + // This write produces the next generation for the key. + this.contextProxy.getStateWriteGeneration(key as never) + 1, + ]), + ) as Record + try { + await this.contextProxy.setValues(sanitizedValues) + } catch (error) { + // A multi-key write can commit some keys before another rejects: put the captured + // previous values back so shared state does not stay half-applied. Route through the + // same guarded restore the failed per-view write below uses - a key that a concurrent + // provider has since written must not be clobbered by this rollback. + await this._restoreSharedValuesAfterFailedViewWrite(previousSharedValues, sanitizedValues, writeGenerations) + throw error + } + try { + await this._saveViewLocalStateFromMutation(sanitizedValues) + } catch (error) { + // The per-view record did not land: restore the prior shared values so the two + // persistence layers cannot diverge, then surface the original failure. + await this._restoreSharedValuesAfterFailedViewWrite(previousSharedValues, sanitizedValues, writeGenerations) + throw error + } + } + + /** Persist the durable subset first, then update the in-memory buffer. */ + private async _saveViewLocalStateFromMutation( + values: Partial & Partial, + ): Promise { + const generation = this.viewLocalStateGeneration + await this._persistViewLocalStateFromMutation(values) + // A reset that ran while the durable write was pending cleared the buffer: applying + // the pre-reset value now would resurrect it. + if (this.viewLocalStateGeneration !== generation) { + 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) { + await this.savePersistedViewState(persistedValues) + } + } + + /** Clear the buffer so getState() falls back to the shared values. */ + private _clearViewLocalState(): void { + this.viewLocalStateGeneration += 1 + this.viewLocalState = {} } // dev @@ -3744,6 +4615,19 @@ export class ClineProvider } await this.contextProxy.resetAllState() + + // Clear the view-local buffer of EVERY live view, not only this one. getState() reads the + // view-local buffer before the shared store, so a sibling view would otherwise keep serving + // its pre-reset mode while the shared state says the reset happened. + for (const instance of ClineProvider.getAllInstances()) { + instance._clearViewLocalState() + } + this._clearViewLocalState() + + // Clear this view's persisted entry too, so the reset selections are not + // re-applied from the durable viewStates pin after a reload. + await this.clearPersistedViewState() + await this.providerSettingsManager.resetAllConfigs() await this.customModesManager.resetCustomModes() await this.removeClineFromStack() @@ -4272,6 +5156,14 @@ export class ClineProvider configuration: RooCodeSettings = {}, ): Promise { if (configuration) { + // Register custom modes before setValues: setValues validates `mode` against + // getCustomModes(), so a mode supplied in the same call would otherwise be dropped. + if (configuration.customModes?.length) { + for (const mode of configuration.customModes) { + await this.customModesManager.updateCustomMode(mode.slug, mode) + } + } + await this.setValues(configuration) if (configuration.allowedCommands) { @@ -4299,16 +5191,6 @@ export class ClineProvider if (configuration.currentApiConfigName) { await this.setProviderProfile(configuration.currentApiConfigName) } - - // Register custom modes so the CustomModesManager knows about them. - // setValues writes to global state, but the manager overwrites that - // when it merges .roomodes + global settings on refresh. Persisting - // via updateCustomMode ensures modes survive the merge cycle. - if (configuration.customModes?.length) { - for (const mode of configuration.customModes) { - await this.customModesManager.updateCustomMode(mode.slug, mode) - } - } } const { @@ -5481,4 +6363,100 @@ 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] + // ContextProxy does not count every key: secrets go through storeSecret and pass-through + // keys straight to the Memento, and neither advances the per-key write generation, so the + // expectation this setter recorded for them (generation 1 against a stored 0) can never + // match and would exempt those keys from the rollback of their own failed attempt. + const uncountedHere = isSecretStateKey(key) || isPassThroughStateKey(key) + if ( + this.contextProxy.getValue(key) === attempted[key] && + (generation === undefined || + uncountedHere || + 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 97a25a7038..2359abd869 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -34,7 +34,13 @@ import { WorkspaceIndexingEnablementManager } from "../../../services/code-index import { Task, TaskOptions } from "../../task/Task" import { safeWriteJson } from "../../../utils/safeWriteJson" +import { t } from "../../../i18n" import { ClineProvider } from "../ClineProvider" + +// The re-adopted cases below swap ClineProvider's readonly members for test doubles. A readonly +// property can only be re-bound through a structural view, so the shape is declared once here +// instead of repeating a double assertion at every stub site (AGENTS.md: no new any casts). +type MemberSwapTarget = { customModesManager: unknown; providerSettingsManager: unknown } import { webviewMessageHandler } from "../webviewMessageHandler" import { Terminal } from "../../../integrations/terminal/Terminal" import { MessageManager } from "../../message-manager" @@ -425,6 +431,37 @@ afterAll(() => { vi.restoreAllMocks() }) +type StalledProfile = { + name: string + apiProvider?: string + openRouterModelId?: string +} + +function stallProviderSettingsProfile(provider: ClineProvider) { + let resolveProfile: (value: StalledProfile) => void = () => {} + const getProfileSpy = vi.fn(() => { + // Only one lookup is resolvable: resolveProfile is bound to the first + // promise's resolver, so a second getProfile would strand its promise. + // Fail loudly instead of hanging. + 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 a stable wrapper around the closure binding: resolveProfile is + // reassigned to the pending promise's resolver once getProfile is called, + // so returning the variable directly would hand the test the initial no-op. + 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) { @@ -571,7 +608,7 @@ describe("ClineProvider", () => { updateGlobalStateSpy = vi.spyOn(provider.contextProxy, "setValue") // @ts-ignore - Accessing private property for testing. - provider.customModesManager = mockCustomModesManager + ;(provider as MemberSwapTarget).customModesManager = mockCustomModesManager // Mock getMcpHub method for generateSystemPrompt provider.getMcpHub = vi.fn().mockReturnValue({ @@ -4570,13 +4607,13 @@ describe("ClineProvider", () => { apiProvider: providerIdentifiers.anthropic, } - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("test-id"), listConfig: vi.fn().mockResolvedValue([profile]), activateProfile: vi.fn().mockResolvedValue(profile), setModeConfig: vi.fn(), getProfile: vi.fn().mockResolvedValue(profile), - } as any + } // Switch to architect mode await messageHandler({ type: "mode", text: "architect" }) @@ -4591,7 +4628,7 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi .fn() @@ -4599,7 +4636,7 @@ describe("ClineProvider", () => { { name: "current-config", id: "current-id", apiProvider: providerIdentifiers.anthropic }, ]), setModeConfig: vi.fn(), - } as any + } await provider.setValue("currentApiConfigName", "current-config") @@ -4620,12 +4657,12 @@ describe("ClineProvider", () => { name: "new-config", } - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { activateProfile: vi.fn().mockResolvedValue(profile), listConfig: vi.fn().mockResolvedValue([profile]), setModeConfig: vi.fn(), getModeConfigId: vi.fn().mockResolvedValue(undefined), - } as any + } // First set the mode await messageHandler({ type: "mode", text: "architect" }) @@ -4647,12 +4684,12 @@ describe("ClineProvider", () => { apiProvider: providerIdentifiers.anthropic, } - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { activateProfile: vi.fn().mockResolvedValue(profile), listConfig: vi.fn().mockResolvedValue([profile]), setModeConfig: vi.fn(), getModeConfigId: vi.fn().mockResolvedValue(undefined), - } as any + } // First set the mode await messageHandler({ type: "mode", text: "architect" }) @@ -4823,7 +4860,7 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi .fn() .mockResolvedValue([ @@ -4831,7 +4868,7 @@ describe("ClineProvider", () => { ]), saveConfig: vi.fn().mockResolvedValue("test-id"), setModeConfig: vi.fn(), - } as any + } // Update API configuration await messageHandler({ @@ -5173,13 +5210,13 @@ describe("ClineProvider", () => { apiProvider: providerIdentifiers.anthropic, } - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("saved-config-id"), listConfig: vi.fn().mockResolvedValue([profile]), activateProfile: vi.fn().mockResolvedValue(profile), setModeConfig: vi.fn(), getProfile: vi.fn().mockResolvedValue(profile), - } as any + } // Switch to architect mode await provider.handleModeSwitch("architect") @@ -5197,7 +5234,7 @@ describe("ClineProvider", () => { }) test("saves current config when switching to mode without config", async () => { - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi .fn() @@ -5205,7 +5242,7 @@ describe("ClineProvider", () => { { name: "current-config", id: "current-id", apiProvider: providerIdentifiers.anthropic }, ]), setModeConfig: vi.fn(), - } as any + } // Mock the ContextProxy's getValue method to return the current config name const contextProxy = (provider as any).contextProxy @@ -5227,6 +5264,55 @@ describe("ClineProvider", () => { // Verify state was posted to webview expect(mockPostMessage).toHaveBeenCalledWith(expect.objectContaining({ type: "state" })) }) + + it("does not read mode config or activate a profile when the mutation is already aborted", async () => { + const getModeConfigId = vi.fn().mockResolvedValue("saved-config-id") + const listConfig = vi.fn().mockResolvedValue([]) + const activateProfile = vi.fn().mockResolvedValue({}) + + ;(provider as MemberSwapTarget).providerSettingsManager = { + getModeConfigId, + listConfig, + activateProfile, + setModeConfig: vi.fn(), + getProfile: vi.fn(), + } + + const controller = new AbortController() + controller.abort() + + await provider["handleModeSwitchUnlocked"]("architect", undefined, controller.signal) + + expect(getModeConfigId).not.toHaveBeenCalled() + expect(activateProfile).not.toHaveBeenCalled() + }) + + it("stops before activating a profile when the mutation aborts during the mode config reads", async () => { + const profile: ProviderSettingsEntry = { + name: "saved-config", + id: "saved-config-id", + apiProvider: providerIdentifiers.anthropic, + } + const controller = new AbortController() + const getModeConfigId = vi.fn().mockResolvedValue("saved-config-id") + const listConfig = vi.fn().mockImplementation(async () => { + controller.abort() + return [profile] + }) + const activateProfile = vi.fn().mockResolvedValue(profile) + + ;(provider as MemberSwapTarget).providerSettingsManager = { + getModeConfigId, + listConfig, + activateProfile, + setModeConfig: vi.fn(), + getProfile: vi.fn().mockResolvedValue(profile), + } + + await provider["handleModeSwitchUnlocked"]("architect", undefined, controller.signal) + + expect(activateProfile).not.toHaveBeenCalled() + }) }) describe("createTaskWithHistoryItem mode validation", () => { @@ -5245,7 +5331,7 @@ describe("ClineProvider", () => { ]), dispose: vi.fn(), } - ;(provider as any).customModesManager = mockCustomModesManager + ;(provider as MemberSwapTarget).customModesManager = mockCustomModesManager // Mock getModeBySlug to return undefined for non-existent mode const { getModeBySlug } = await import("../../../shared/modes") @@ -5259,7 +5345,7 @@ describe("ClineProvider", () => { }) // Subsequent calls return default mode // Mock provider settings manager - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), } @@ -5287,7 +5373,12 @@ describe("ClineProvider", () => { expect(getModeBySlug).toHaveBeenCalledWith("non-existent-mode", expect.any(Array)) // Verify fallback to default mode - expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "code") + expect(mockContext.globalState.update).toHaveBeenCalledWith("viewStates", expect.anything()) + // This branch persists the restored mode through the view's own pin rather than the shared + // global write main asserted on, because a global write would leak the restored mode into + // other views in parallel mode. Same claim, moved observable: the resolved mode is what + // this view now holds, and historyItem still carries it below. + expect(provider["viewLocalState"].mode).toBe("code") expect(logSpy).toHaveBeenCalledWith( "Mode 'non-existent-mode' from history no longer exists. Falling back to default mode 'code'.", ) @@ -5311,7 +5402,7 @@ describe("ClineProvider", () => { ]), dispose: vi.fn(), } - ;(provider as any).customModesManager = mockCustomModesManager + ;(provider as MemberSwapTarget).customModesManager = mockCustomModesManager // Mock getModeBySlug to return the custom mode const { getModeBySlug } = await import("../../../shared/modes") @@ -5323,7 +5414,7 @@ describe("ClineProvider", () => { }) // Mock provider settings manager - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("config-id"), listConfig: vi .fn() @@ -5360,7 +5451,12 @@ describe("ClineProvider", () => { expect(getModeBySlug).toHaveBeenCalledWith("custom-mode", expect.any(Array)) // Verify mode was preserved - expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "custom-mode") + expect(mockContext.globalState.update).toHaveBeenCalledWith("viewStates", expect.anything()) + // This branch persists the restored mode through the view's own pin rather than the shared + // global write main asserted on, because a global write would leak the restored mode into + // other views in parallel mode. Same claim, moved observable: the resolved mode is what + // this view now holds, and historyItem still carries it below. + expect(provider["viewLocalState"].mode).toBe("custom-mode") expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("no longer exists")) // Verify history item mode was not changed @@ -5375,7 +5471,7 @@ describe("ClineProvider", () => { getCustomModes: vi.fn().mockResolvedValue([]), dispose: vi.fn(), } - ;(provider as any).customModesManager = mockCustomModesManager + ;(provider as MemberSwapTarget).customModesManager = mockCustomModesManager // Mock getModeBySlug to return built-in architect mode const { getModeBySlug } = await import("../../../shared/modes") @@ -5387,7 +5483,7 @@ describe("ClineProvider", () => { }) // Mock provider settings manager - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), } @@ -5408,7 +5504,12 @@ describe("ClineProvider", () => { await provider.createTaskWithHistoryItem(historyItem) // Verify mode was preserved - expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "architect") + expect(mockContext.globalState.update).toHaveBeenCalledWith("viewStates", expect.anything()) + // This branch persists the restored mode through the view's own pin rather than the shared + // global write main asserted on, because a global write would leak the restored mode into + // other views in parallel mode. Same claim, moved observable: the resolved mode is what + // this view now holds, and historyItem still carries it below. + expect(provider["viewLocalState"].mode).toBe("architect") // Verify history item mode was not changed expect(historyItem.mode).toBe("architect") @@ -5418,7 +5519,7 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) // Mock provider settings manager - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), } @@ -5450,7 +5551,7 @@ describe("ClineProvider", () => { getCustomModes: vi.fn().mockResolvedValue([]), dispose: vi.fn(), } - ;(provider as any).customModesManager = mockCustomModesManager + ;(provider as MemberSwapTarget).customModesManager = mockCustomModesManager // Mock getModeBySlug to return built-in mode const { getModeBySlug } = await import("../../../shared/modes") @@ -5462,7 +5563,7 @@ describe("ClineProvider", () => { }) // Mock provider settings manager to throw error - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("config-id"), listConfig: vi .fn() @@ -5497,6 +5598,174 @@ describe("ClineProvider", () => { }) }) + describe("deleteProviderProfile", () => { + it("moves the settings manager's current profile to the survivor when the active profile is deleted", async () => { + const contextProxy = new ContextProxy(mockContext) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + const profileToDelete: ProviderSettingsEntry = { name: "deleted", id: "deleted-id" } + + await contextProxy.setValue("listApiConfigMeta", [ + { name: "deleted", id: "deleted-id" }, + { name: "survivor", id: "survivor-id" }, + ]) + await contextProxy.setValue("currentApiConfigName", "deleted") + + const activateProfile = vi.fn().mockResolvedValue({ name: "survivor" }) + + ;(provider as MemberSwapTarget).providerSettingsManager = { + deleteConfig: vi.fn().mockResolvedValue(undefined), + activateProfile, + getProfile: vi.fn().mockResolvedValue({ + name: "survivor", + apiProvider: providerIdentifiers.anthropic, + }), + } + + await provider.deleteProviderProfile(profileToDelete) + + expect(activateProfile).toHaveBeenCalledWith({ name: "survivor" }) + + await provider.dispose() + }) + + it("clears the settings overlay of a view re-pinned when the survivor's settings cannot be resolved", async () => { + const contextProxy = new ContextProxy(mockContext) + const deleter = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + const pinned = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const profileToDelete: ProviderSettingsEntry = { name: "deleted", id: "deleted-id" } + const deletedSettings = { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "deleted-model" } + + await contextProxy.setValue("listApiConfigMeta", [ + { name: "deleted", id: "deleted-id" }, + { name: "survivor", id: "survivor-id" }, + ]) + await contextProxy.setValue("currentApiConfigName", "deleted") + await contextProxy.setProviderSettings(deletedSettings) + + for (const view of [deleter, pinned]) { + await view.saveViewState("currentApiConfigName", "deleted") + // The buffer a completed loadViewState leaves behind: this view serves the deleted + // profile's configuration. + await view["_saveViewLocalStateFromMutation"]({ apiConfiguration: deletedSettings }) + } + + vi.spyOn(deleter["providerSettingsManager"], "deleteConfig").mockResolvedValue(undefined) + vi.spyOn(deleter["providerSettingsManager"], "activateProfile").mockResolvedValue({ + name: "survivor", + id: "survivor-id", + apiProvider: providerIdentifiers.anthropic, + }) + vi.spyOn(deleter["providerSettingsManager"], "getProfile").mockRejectedValue( + new Error("profile store unavailable"), + ) + + await deleter.deleteProviderProfile(profileToDelete) + + // Both views are re-pointed at the survivor, so neither may keep serving the deleted + // profile's configuration under the survivor's name. + expect(deleter["viewLocalState"].currentApiConfigName).toBe("survivor") + expect(deleter["viewLocalState"]).not.toHaveProperty("apiConfiguration") + expect(pinned["viewLocalState"].currentApiConfigName).toBe("survivor") + expect(pinned["viewLocalState"]).not.toHaveProperty("apiConfiguration") + + await deleter.dispose() + await pinned.dispose() + }) + + it("rebuilds a live task that still runs on the deleted profile, in this view and in a sibling view", async () => { + const contextProxy = new ContextProxy(mockContext) + const deleter = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + const sibling = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const profileToDelete: ProviderSettingsEntry = { name: "deleted", id: "deleted-id" } + const survivingSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "survivor-key" } + + await contextProxy.setValue("listApiConfigMeta", [ + { name: "deleted", id: "deleted-id" }, + { name: "survivor", id: "survivor-id" }, + ]) + await contextProxy.setValue("currentApiConfigName", "deleted") + + const views = [deleter, sibling] + const tasks = views.map((view) => { + // The double only backs the calls the delete path makes on a live task. + const task = new Task(defaultTaskOptions) + Object.defineProperty(task, "taskId", { value: `task-of-${view.viewId}`, writable: true }) + Object.defineProperty(task, "taskApiConfigName", { value: "deleted", writable: true }) + Object.defineProperty(task, "apiConfiguration", { + value: { apiProvider: providerIdentifiers.openrouter, openRouterModelId: "deleted-model" }, + writable: true, + }) + task.setTaskApiConfigName = vi.fn() + task.updateApiConfiguration = vi.fn() + return task + }) + + for (const [index, view] of views.entries()) { + await view.saveViewState("currentApiConfigName", "deleted") + await view.addClineToStack(tasks[index]) + } + + vi.spyOn(deleter["providerSettingsManager"], "deleteConfig").mockResolvedValue(undefined) + vi.spyOn(deleter["providerSettingsManager"], "activateProfile").mockResolvedValue({ + name: "survivor", + id: "survivor-id", + apiProvider: providerIdentifiers.anthropic, + }) + vi.spyOn(deleter["providerSettingsManager"], "getProfile").mockResolvedValue({ + name: "survivor", + id: "survivor-id", + ...survivingSettings, + }) + + await deleter.deleteProviderProfile(profileToDelete) + + // Re-pointing the view is not enough: a task that still names the deleted profile runs on a + // handler built from its settings and keeps it as its sticky profile until it is rebuilt. + for (const task of tasks) { + expect(task.updateApiConfiguration).toHaveBeenCalledWith( + expect.objectContaining({ + apiProvider: providerIdentifiers.anthropic, + apiKey: "survivor-key", + }), + ) + expect(task.setTaskApiConfigName).toHaveBeenCalledWith("survivor") + } + + await deleter.dispose() + await sibling.dispose() + }) + }) + describe("updateCustomMode", () => { test("updates both file and state when updating custom mode", async () => { await provider.resolveWebviewView(mockWebviewView) @@ -5564,14 +5833,14 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { setModeConfig: vi.fn().mockRejectedValue(new Error("Failed to update mode config")), listConfig: vi .fn() .mockResolvedValue([ { name: "test-config", id: "test-id", apiProvider: providerIdentifiers.anthropic }, ]), - } as any + } // Mock getState to provide necessary data vi.spyOn(provider, "getState").mockResolvedValue({ @@ -5597,7 +5866,7 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { setModeConfig: vi.fn(), saveConfig: vi.fn().mockResolvedValue(undefined), listConfig: vi @@ -5605,7 +5874,7 @@ describe("ClineProvider", () => { .mockResolvedValue([ { name: "test-config", id: "test-id", apiProvider: providerIdentifiers.anthropic }, ]), - } as any + } const testApiConfig = { apiProvider: providerIdentifiers.anthropic, @@ -5642,7 +5911,7 @@ describe("ClineProvider", () => { ;(buildApiHandler as any).mockImplementationOnce(() => { throw new Error("API handler error") }) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { setModeConfig: vi.fn(), saveConfig: vi.fn().mockResolvedValue(undefined), listConfig: vi @@ -5650,7 +5919,7 @@ describe("ClineProvider", () => { .mockResolvedValue([ { name: "test-config", id: "test-id", apiProvider: providerIdentifiers.anthropic }, ]), - } as any + } // Setup Task instance with auto-mock from the top of the file const mockCline = new Task(defaultTaskOptions) // Create a new mocked instance @@ -5685,7 +5954,7 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { setModeConfig: vi.fn(), saveConfig: vi.fn().mockResolvedValue(undefined), listConfig: vi @@ -5693,7 +5962,7 @@ describe("ClineProvider", () => { .mockResolvedValue([ { name: "test-config", id: "test-id", apiProvider: providerIdentifiers.anthropic }, ]), - } as any + } const testApiConfig = { apiProvider: providerIdentifiers.anthropic, @@ -5719,124 +5988,1651 @@ describe("ClineProvider", () => { ]) }) }) -}) -describe("webviewMessageHandler no-floating-promises coverage", () => { - const createProvider = (overrides: Record = {}) => - Object.assign( - { - context: { - secrets: { - get: vi.fn().mockResolvedValue(undefined), - }, - }, - contextProxy: { - getValue: vi.fn(), - setValue: vi.fn().mockResolvedValue(undefined), - }, - postMessageToWebview: vi.fn().mockResolvedValue(true), - postStateToWebview: vi.fn().mockResolvedValue(undefined), - getCurrentTask: vi.fn(), - getCurrentWorkspaceCodeIndexScope: vi.fn(), - getMcpHub: vi.fn().mockReturnValue({ - getMcpSettingsFilePath: vi.fn().mockResolvedValue("/test/mcp.json"), - }), - providerSettingsManager: { - listConfig: vi.fn().mockResolvedValue([]), - }, - customModesManager: { - getCustomModesFilePath: vi.fn().mockResolvedValue("/test/custom-modes.yaml"), - exportModeWithRules: vi.fn(), - importModeWithRules: vi.fn(), - getCustomModes: vi.fn().mockResolvedValue([]), - checkRulesDirectoryHasContent: vi.fn().mockResolvedValue(true), - }, - exportTaskWithId: vi.fn().mockResolvedValue(undefined), - showTaskWithId: vi.fn().mockResolvedValue(undefined), - condenseTaskContext: vi.fn().mockResolvedValue(undefined), - deleteTaskWithId: vi.fn().mockResolvedValue(undefined), - log: vi.fn(), - cwd: "/test/workspace", - }, - overrides, - ) as unknown as 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(), + ) - const createIndexManager = (overrides: Record = {}) => - Object.assign( - { - setWorkspaceEnabled: vi.fn().mockResolvedValue(undefined), - setAutoEnableDefault: vi.fn().mockResolvedValue(undefined), - isFeatureEnabled: true, - isFeatureConfigured: true, - isWorkspaceEnabled: true, - initialize: vi.fn().mockResolvedValue(undefined), - state: "Standby", - isInitialized: true, - startIndexing: vi.fn().mockResolvedValue(undefined), - stopIndexing: vi.fn(), - clearIndexData: vi.fn().mockResolvedValue(undefined), - getCurrentStatus: vi.fn().mockReturnValue({ systemStatus: "Standby" }), - }, - overrides, - ) + await provider["setViewStateId"]("__proto__") - beforeEach(() => { - vi.clearAllMocks() - }) + // "__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({}) - it("logs a detached indexing rejection without rejecting the handler", async () => { - let rejectIndexing!: (error: Error) => void - const indexingPromise = new Promise((_, reject) => { - rejectIndexing = reject - }) - const manager = createIndexManager({ - startIndexing: vi.fn().mockReturnValue(indexingPromise), - }) - const provider = createProvider({ - getCurrentWorkspaceCodeIndexScope: vi.fn().mockReturnValue({ codeIndexManager: manager }), + await provider.dispose() }) - await expect(webviewMessageHandler(provider, { type: "startIndexing" })).resolves.toBeUndefined() - expect(provider.getCurrentWorkspaceCodeIndexScope).toHaveBeenCalledOnce() - expect(manager.startIndexing).toHaveBeenCalledOnce() - - rejectIndexing(new Error("boom")) - await Promise.resolve() - await Promise.resolve() + 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) + + // 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) }, + }) - expect(provider.log).toHaveBeenCalledWith("Indexing error: Error: boom") + await provider.dispose() + }) }) - it("covers the changed task-operation happy paths", async () => { - const task = { - taskId: "task-1", - handleTerminalOperation: vi.fn().mockResolvedValue(undefined), - } - const provider = createProvider({ getCurrentTask: vi.fn().mockReturnValue(task) }) - - await webviewMessageHandler(provider, { type: "terminalOperation", terminalOperation: "continue" }) - await webviewMessageHandler(provider, { type: "exportCurrentTask" }) - await webviewMessageHandler(provider, { type: "showTaskWithId", text: "task-2" }) - await webviewMessageHandler(provider, { type: "condenseTaskContextRequest", text: "task-2" }) - await webviewMessageHandler(provider, { type: "deleteTaskWithId", text: "task-2" }) - await webviewMessageHandler(provider, { type: "exportTaskWithId", text: "task-2" }) + 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 }, + }) + }) - expect(task.handleTerminalOperation).toHaveBeenCalledWith("continue") - expect(provider.exportTaskWithId).toHaveBeenCalledTimes(2) - expect(provider.showTaskWithId).toHaveBeenCalledWith("task-2") - expect(provider.condenseTaskContext).toHaveBeenCalledWith("task-2") - expect(provider.deleteTaskWithId).toHaveBeenCalledWith("task-2") - }) + await provider["setViewStateId"]("legacy-view") - it("covers changed file, image, mention, settings, and TTS dispatch paths", async () => { - const { openFile } = await import("../../../integrations/misc/open-file") - const { openImage, saveImage } = await import("../../../integrations/misc/image-handler") - const { openMention } = await import("../../mentions") - const { playTts } = await import("../../../utils/tts") - const provider = createProvider() + // 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() + }) - await webviewMessageHandler(provider, { type: "openImage", text: "/test/image.png" }) - await webviewMessageHandler(provider, { type: "saveImage", dataUri: "invalid" }) + 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(), + ) + // A string in storage is corrupt: the fresh-read guard must not spread it. + 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 is the author of 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 not pin the persisted profile name 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") + // The name is pinned only with its resolved configuration: otherwise getState() + // would pair the missing profile's name with unrelated shared settings. + expect(provider["viewLocalState"]).not.toHaveProperty("currentApiConfigName") + 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 as MemberSwapTarget).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 as MemberSwapTarget).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() + }) + }) + + 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 exists and the proxy's global-state cache is empty + // (initialize() is never called in this fixture; only "taskHistory" passes + // through to the context store), so getState() falls back to the shared + // defaults: mode "code" (defaultModeSlug) and currentApiConfigName "default". + 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() + }) + + it("keeps the entry of a live view outside the 50-entry cutoff", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider["setViewStateId"]("live-view") + + const states: Record = Object.fromEntries( + Array.from({ length: 55 }, (_, index) => [ + `closed-${index}`, + { mode: `mode-${index}`, updatedAt: index + 1 }, + ]), + ) + // The live view's entry is the oldest and would be evicted by the plain cutoff. + states["live-view"] = { mode: "architect", updatedAt: 0 } + + const pruned = provider["prunePersistedViewStates"](states) + + expect(pruned["live-view"]).toEqual({ mode: "architect", updatedAt: 0 }) + expect(Object.keys(pruned)).toHaveLength(51) + + await provider.dispose() + }) + }) + + describe("view-local buffer: loading, setValue mutations and rollback", () => { + 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() + }) + }) + + it("should persist known modes and reject unknown or non-string modes", 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 as MemberSwapTarget).customModesManager = { + getCustomModes: vi.fn().mockResolvedValue([]), + dispose: vi.fn(), + } + // The file-level modes mock resolves every slug to a mode; narrow it to the slugs under test. + const modesModule = vi.mocked(await import("../../../shared/modes")) + const originalMode = modesModule.getModeBySlug("code") + modesModule.getModeBySlug.mockImplementation(((slug: string) => + slug === "refactor" ? { slug } : undefined) as typeof modesModule.getModeBySlug) + try { + await provider.setValues({ mode: "refactor" }) + expect(mockContext.globalState.get("mode")).toBe("refactor") + expect(provider["viewLocalState"].mode).toBe("refactor") + await provider.setValues({ mode: "bogus-mode" }) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('Ignoring invalid mode "bogus-mode"')) + expect(mockContext.globalState.get("mode")).toBe("refactor") + expect(provider["viewLocalState"].mode).toBe("refactor") + // A non-string mode must be rejected before persistence; Object.assign + // keeps this a single structural assertion, so it must not reach global + // state or the buffer. + await provider.setValues(Object.assign({} as RooCodeSettings, { mode: 42 })) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining('Ignoring invalid mode "42"')) + expect(mockContext.globalState.get("mode")).toBe("refactor") + expect(provider["viewLocalState"].mode).toBe("refactor") + } finally { + modesModule.getModeBySlug.mockReturnValue(originalMode) + } + await provider.dispose() + }) + + it("should build the buffered apiConfiguration from provider settings keys", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + provider["viewLocalState"] = { apiConfiguration: { openRouterApiKey: "key-1" } } + await provider.setValues({ apiProvider: providerIdentifiers.openrouter }) + expect(provider["viewLocalState"].apiConfiguration).toStrictEqual({ + apiProvider: providerIdentifiers.openrouter, + }) + await provider.setValues({ openRouterModelId: "model-x" }) + expect(provider["viewLocalState"].apiConfiguration).toEqual({ + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "model-x", + }) + await provider.dispose() + }) + + it("should clear viewLocalState and the persisted entry when resetting state", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + // @ts-ignore - Replace customModesManager with a test double (the real reset writes to disk). + ;(provider as MemberSwapTarget).customModesManager = { + resetCustomModes: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn(), + } + // The modal answer is a string label; the last-typed vscode overload expects a + // MessageItem. A double assertion is the last-resort cast here (AGENTS.md): the + // production path compares the answer against the string label directly, so the + // runtime value must stay a string and cannot be a structural MessageItem double. + vi.mocked(vscode.window.showInformationMessage).mockResolvedValue( + t("common:answers.yes") as unknown as vscode.MessageItem, + ) + await provider["setViewStateId"]("stable-sidebar-view") + await provider.saveViewState("mode", "architect") + await provider.resetState() + expect(provider["viewLocalState"]).toEqual({}) + expect(mockContext.globalState.get("viewStates")).toEqual({}) + await provider.dispose() + }) + + it("should discard a load that finishes after reset cleared the buffer", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider["setViewStateId"]("stable-sidebar-view") + await provider.contextProxy.setValue("viewStates", { + "stable-sidebar-view": { mode: "architect", updatedAt: 1 }, + }) + + let releaseModes: () => void = () => {} + vi.spyOn(provider.customModesManager, "getCustomModes").mockImplementation( + () => + new Promise((resolve) => { + releaseModes = () => resolve([]) + }), + ) + + const load = provider["loadViewState"]() + // The reset clears the buffer while the load awaits the mode lookup. + provider["_clearViewLocalState"]() + releaseModes() + await load + + expect(provider["viewLocalState"]).toEqual({}) + await provider.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(), + ) + await pinned.setValue("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; activating the profile must + // refresh that copy, 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 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(), + ) + 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) + + // A save that does not activate still rewrites the profile on disk. Every other live view + // pinned to that profile buffered a copy of its settings, so without a refresh this save + // leaves them serving the previous token under the same profile name. + expect(pinned["viewLocalState"].apiConfiguration).toEqual(providerSettings) + + await saver.dispose() + await pinned.dispose() + }) + + it("should rebuild a sibling task that follows the saved profile even when its view pins another", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const sibling = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // A task restored from history can follow profile A while its view pins profile B: the pin + // must not be the condition for rebuilding that task's handler. + await sibling["saveViewState"]("currentApiConfigName", "another-profile") + // updateTaskApiHandlerIfNeeded is private to ClineProvider, so the double is bound through a + // structural view; the assertion goes through `never` rather than `any` (AGENTS.md). + const rebuildSpy = vi.fn() + ;(sibling as never as { updateTaskApiHandlerIfNeeded: typeof rebuildSpy }).updateTaskApiHandlerIfNeeded = + rebuildSpy + vi.spyOn(sibling, "getCurrentTask").mockReturnValue({ taskApiConfigName: "shared-profile" } as never) + // @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) + + expect(rebuildSpy).toHaveBeenCalledWith(providerSettings, { forceRebuild: true }) + + await saver.dispose() + await sibling.dispose() + }) + + it("should roll back the shared writes when setValues per-view persistence fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "architect") + await provider.contextProxy.setValue("currentApiConfigName", "current-config") + 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.setValues({ mode: "code", currentApiConfigName: "other-config" })).rejects.toThrow( + "storage unavailable", + ) + expect(provider.getValue("mode")).toBe("architect") + expect(provider.getValue("currentApiConfigName")).toBe("current-config") + expect(provider["viewLocalState"].mode).toBeUndefined() + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + + it("should not roll back a shared key that a newer write replaced during a partial setValues", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "architect") + await provider.contextProxy.setValue("currentApiConfigName", "current-config") + const originalUpdate = mockContext.globalState.update + let rejectConfig: (error: Error) => void = () => {} + mockContext.globalState.update = vi.fn().mockImplementation((key: string) => { + if (key === "currentApiConfigName") { + return new Promise((_, reject) => (rejectConfig = reject)) + } + return Promise.resolve() + }) + + // The mode write commits while the profile-name write is still in flight. + const attempt = provider.setValues({ mode: "code", currentApiConfigName: "other-config" }) + await vi.waitFor(() => expect(provider.getValue("mode")).toBe("code")) + // A newer write for the already committed key lands before the rollback runs. + await provider.setValue("mode", "plan") + rejectConfig(new Error("storage unavailable")) + await expect(attempt).rejects.toThrow("storage unavailable") + + // A rollback may only undo keys that still hold the value this attempt wrote; replaying + // every captured value would clobber the newer write for every view that reads shared state. + expect(provider.getValue("mode")).toBe("plan") + expect(provider.getValue("currentApiConfigName")).toBe("current-config") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + it("should roll back a secret key that ContextProxy does not track with a write generation", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("nanoGptApiKey", "old-secret") + await provider.contextProxy.setValue("mode", "architect") + const secretStore = vi.spyOn(mockContext.secrets, "store") + 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.setValues({ nanoGptApiKey: "new-secret", mode: "code" })).rejects.toThrow( + "storage unavailable", + ) + + // ContextProxy routes secret keys through storeSecret, which never advances the per-key + // write generation the rollback guard compares against. A key it does not count is captured + // with generation 1 and still reads 0 afterwards, so the guard excludes it and the secret + // alone stays at the value the failed attempt wrote while every other key is restored. + // The attempt did move the secret: ContextProxy.storeSecret is fire-and-forget, so + // only the presence of the attempted value and the restored end state are deterministic. + expect(secretStore.mock.calls.map(([key, value]) => [key, value])).toContainEqual([ + "nanoGptApiKey", + "new-secret", + ]) + expect(provider.getValue("mode")).toBe("architect") + expect(provider.contextProxy.getSecret("nanoGptApiKey")).toBe("old-secret") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + + it("should keep the generation guard for a counted key whose concurrent write carried the same value", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "architect") + await provider.contextProxy.setValue("currentApiConfigName", "current-config") + const originalUpdate = mockContext.globalState.update + let rejectViews: (error: Error) => void = () => {} + let viewWrites = 0 + mockContext.globalState.update = vi.fn().mockImplementation((key: string) => { + if (key === "viewStates") { + // The attempt's own per-view write is parked below; the concurrent write's must land. + viewWrites++ + if (viewWrites === 1) { + return new Promise((_, reject) => (rejectViews = reject)) + } + return Promise.resolve() + } + return Promise.resolve() + }) + + const attempt = provider.setValues({ mode: "code", currentApiConfigName: "other-config" }) + await vi.waitFor(() => expect(provider.getValue("mode")).toBe("code")) + // A concurrent provider re-writes the very value this attempt wrote: the stored value + // cannot tell the two writes apart, only the per-key generation can. + await provider.contextProxy.setValue("mode", "code") + rejectViews(new Error("storage unavailable")) + await expect(attempt).rejects.toThrow("storage unavailable") + + // Restoring the captured "architect" here would undo a newer write that this rollback + // did not make, for every view that reads shared state. + expect(provider.getValue("mode")).toBe("code") + expect(provider.getValue("currentApiConfigName")).toBe("current-config") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + + it("should roll back a pass-through key that ContextProxy hands straight to the Memento", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const firstHistory = [ + { + id: "task-1", + number: 1, + ts: 1700000000000, + task: "first", + tokensIn: 1, + tokensOut: 1, + totalCost: 0, + }, + ] + const secondHistory = [ + { + id: "task-2", + number: 2, + ts: 1700000000001, + task: "second", + tokensIn: 2, + tokensOut: 2, + totalCost: 0, + }, + ] + await provider.contextProxy.setValue("taskHistory", firstHistory as never) + await provider.contextProxy.setValue("mode", "architect") + const originalUpdate = mockContext.globalState.update + // The double must still move the Memento: a pass-through key is read back from there, + // so replacing the store outright would hide the very write under test. + const globalStore = vi.fn().mockImplementation((key: string, value: unknown) => { + if (key === "viewStates") { + return Promise.reject(new Error("storage unavailable")) + } + return originalUpdate(key, value as never) + }) + mockContext.globalState.update = globalStore + + await expect(provider.setValues({ taskHistory: secondHistory as never, mode: "code" })).rejects.toThrow( + "storage unavailable", + ) + + // taskHistory is a pass-through key: ContextProxy writes it straight to the Memento and + // never advances its write generation, so a guard that demands one leaves the durable + // history at the value the failed attempt wrote. + // taskHistory is a pass-through key: ContextProxy writes it straight to the Memento and + // never advances its write generation, so a guard that still demands one for it leaves + // the durable history at the value the failed attempt wrote while every other key is + // restored. The write sequence is the observable that separates a restored key from + // one the rollback never touched. + expect(globalStore.mock.calls.filter(([key]) => key === "taskHistory").map(([, value]) => value)).toEqual([ + secondHistory, + firstHistory, + ]) + expect(provider.getValue("mode")).toBe("architect") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + + it("should serve this view's restored mode through getState()", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "architect") + await provider["setViewStateId"]("mode-view") + // A history or profile restore writes the mode into this view's buffer only; the shared store + // keeps the mode the last committing view chose. + await provider.saveViewState("mode", "code") + + expect(provider.getValue("mode")).toBe("architect") + // getState() is what task creation and getStateToPostToWebview read, so the restored mode has + // to be visible there and not only through getValues(). + expect((await provider.getState()).mode).toBe("code") + + await provider.dispose() + }) + + describe("viewId uniqueness", () => { + it("should assign unique viewId to each instance", 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(), + ) + + // Each instance should have a unique viewId + expect(provider1.viewId).toBeDefined() + expect(provider2.viewId).toBeDefined() + expect(provider1.viewId).not.toBe(provider2.viewId) + + await provider1.dispose() + await provider2.dispose() + }) + + it("should have viewId in correct format: {renderContext}-{instanceCount}", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + expect(provider.viewId).toMatch(/^sidebar-\d+$/) + + await provider.dispose() + }) + + it("should increment instance count for each new instance", async () => { + const provider1 = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const provider2 = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + // First editor instance should be "editor-0" (or next available) + // Second editor instance should have a different number + const num1 = parseInt(provider1.viewId.split("-")[1]!) + const num2 = parseInt(provider2.viewId.split("-")[1]!) + + expect(num2).toBeGreaterThan(num1) + + await provider1.dispose() + await provider2.dispose() + }) + }) + + it("should reapply an independently mutated profile name when the load settles", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const { getProfile: getProfileSpy, resolveProfile } = stallProviderSettingsProfile(provider) + 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)) + // The profile name changes while the lookup is in flight; the load still + // resolves the name persisted at load start. + await provider.saveViewState("currentApiConfigName", "cfg-b") + + resolveProfile({ name: "cfg-a", apiProvider: providerIdentifiers.openrouter, openRouterModelId: "model-x" }) + await load + + // The in-flight selection wins over the loaded state, and cfg-a's settings are not parked + // under cfg-b's name: a name this view changed on its own pins no settings, so reads fall back + // to the shared store until an activation resolves cfg-b together with its configuration. + expect(provider["viewLocalState"].currentApiConfigName).toBe("cfg-b") + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + expect(provider["viewLocalState"]).not.toHaveProperty("mode") + await provider.dispose() + }) + + test("postMessageToWebview does not await the webview ack", async () => { + await provider.resolveWebviewView(mockWebviewView) + + let releaseAck!: () => void + const ack = new Promise((resolve) => { + releaseAck = resolve + }) + mockPostMessage.mockImplementationOnce(() => ack) + + const message: ExtensionMessage = { type: "action", action: "chatButtonClicked" } + + // The caller must not wait for the renderer ack: a webview page remounted or disposed + // while the post is in flight never acknowledges it, and awaiting that promise would + // wedge every caller on the task critical path. + await provider.postMessageToWebview(message) + + expect(mockPostMessage).toHaveBeenCalledWith(message) + releaseAck() + }) + + test("postMessageToWebview logs a synchronous postMessage throw", async () => { + await provider.resolveWebviewView(mockWebviewView) + const logSpy = vi.spyOn(provider, "log").mockImplementation(() => {}) + // A disposed webview can throw synchronously from postMessage, before any + // promise exists to attach the rejection handler to: the call site must handle + // it so fire-and-forget callers never see an unhandled rejection. + mockPostMessage.mockImplementationOnce(() => { + throw new Error("Webview is disposed") + }) + const message: ExtensionMessage = { type: "action", action: "chatButtonClicked" } + await expect(provider.postMessageToWebview(message)).resolves.toBeUndefined() + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining("[postMessageToWebview] dropped message type=action"), + ) + logSpy.mockRestore() + }) +}) + +describe("webviewMessageHandler no-floating-promises coverage", () => { + const createProvider = (overrides: Record = {}) => + Object.assign( + { + context: { + secrets: { + get: vi.fn().mockResolvedValue(undefined), + }, + }, + contextProxy: { + getValue: vi.fn(), + setValue: vi.fn().mockResolvedValue(undefined), + }, + postMessageToWebview: vi.fn().mockResolvedValue(true), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + getCurrentTask: vi.fn(), + getCurrentWorkspaceCodeIndexScope: vi.fn(), + getMcpHub: vi.fn().mockReturnValue({ + getMcpSettingsFilePath: vi.fn().mockResolvedValue("/test/mcp.json"), + }), + providerSettingsManager: { + listConfig: vi.fn().mockResolvedValue([]), + }, + customModesManager: { + getCustomModesFilePath: vi.fn().mockResolvedValue("/test/custom-modes.yaml"), + exportModeWithRules: vi.fn(), + importModeWithRules: vi.fn(), + getCustomModes: vi.fn().mockResolvedValue([]), + checkRulesDirectoryHasContent: vi.fn().mockResolvedValue(true), + }, + exportTaskWithId: vi.fn().mockResolvedValue(undefined), + showTaskWithId: vi.fn().mockResolvedValue(undefined), + condenseTaskContext: vi.fn().mockResolvedValue(undefined), + deleteTaskWithId: vi.fn().mockResolvedValue(undefined), + log: vi.fn(), + cwd: "/test/workspace", + }, + overrides, + ) as unknown as ClineProvider + + const createIndexManager = (overrides: Record = {}) => + Object.assign( + { + setWorkspaceEnabled: vi.fn().mockResolvedValue(undefined), + setAutoEnableDefault: vi.fn().mockResolvedValue(undefined), + isFeatureEnabled: true, + isFeatureConfigured: true, + isWorkspaceEnabled: true, + initialize: vi.fn().mockResolvedValue(undefined), + state: "Standby", + isInitialized: true, + startIndexing: vi.fn().mockResolvedValue(undefined), + stopIndexing: vi.fn(), + clearIndexData: vi.fn().mockResolvedValue(undefined), + getCurrentStatus: vi.fn().mockReturnValue({ systemStatus: "Standby" }), + }, + overrides, + ) + + beforeEach(() => { + vi.clearAllMocks() + }) + + it("logs a detached indexing rejection without rejecting the handler", async () => { + let rejectIndexing!: (error: Error) => void + const indexingPromise = new Promise((_, reject) => { + rejectIndexing = reject + }) + const manager = createIndexManager({ + startIndexing: vi.fn().mockReturnValue(indexingPromise), + }) + const provider = createProvider({ + getCurrentWorkspaceCodeIndexScope: vi.fn().mockReturnValue({ codeIndexManager: manager }), + }) + + await expect(webviewMessageHandler(provider, { type: "startIndexing" })).resolves.toBeUndefined() + expect(provider.getCurrentWorkspaceCodeIndexScope).toHaveBeenCalledOnce() + expect(manager.startIndexing).toHaveBeenCalledOnce() + + rejectIndexing(new Error("boom")) + await Promise.resolve() + await Promise.resolve() + + expect(provider.log).toHaveBeenCalledWith("Indexing error: Error: boom") + }) + + it("covers the changed task-operation happy paths", async () => { + const task = { + taskId: "task-1", + handleTerminalOperation: vi.fn().mockResolvedValue(undefined), + } + const provider = createProvider({ getCurrentTask: vi.fn().mockReturnValue(task) }) + + await webviewMessageHandler(provider, { type: "terminalOperation", terminalOperation: "continue" }) + await webviewMessageHandler(provider, { type: "exportCurrentTask" }) + await webviewMessageHandler(provider, { type: "showTaskWithId", text: "task-2" }) + await webviewMessageHandler(provider, { type: "condenseTaskContextRequest", text: "task-2" }) + await webviewMessageHandler(provider, { type: "deleteTaskWithId", text: "task-2" }) + await webviewMessageHandler(provider, { type: "exportTaskWithId", text: "task-2" }) + + expect(task.handleTerminalOperation).toHaveBeenCalledWith("continue") + expect(provider.exportTaskWithId).toHaveBeenCalledTimes(2) + expect(provider.showTaskWithId).toHaveBeenCalledWith("task-2") + expect(provider.condenseTaskContext).toHaveBeenCalledWith("task-2") + expect(provider.deleteTaskWithId).toHaveBeenCalledWith("task-2") + }) + + it("covers changed file, image, mention, settings, and TTS dispatch paths", async () => { + const { openFile } = await import("../../../integrations/misc/open-file") + const { openImage, saveImage } = await import("../../../integrations/misc/image-handler") + const { openMention } = await import("../../mentions") + const { playTts } = await import("../../../utils/tts") + const provider = createProvider() + + await webviewMessageHandler(provider, { type: "openImage", text: "/test/image.png" }) + await webviewMessageHandler(provider, { type: "saveImage", dataUri: "invalid" }) await webviewMessageHandler(provider, { type: "openFile", text: "/test/file.ts" }) await webviewMessageHandler(provider, { type: "openMention", text: "file.ts" }) await webviewMessageHandler(provider, { type: "openCustomModesSettings" }) @@ -8120,7 +9916,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { const upsertSpy = vi.spyOn(provider, "upsertProviderProfile").mockResolvedValue("profile-id") vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi.fn().mockResolvedValue([]), } @@ -8138,20 +9934,35 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { ) }) + it("logs and posts state when profile persistence fails", async () => { + vi.spyOn(provider, "getState").mockRejectedValue(new Error("state unavailable")) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + ;(provider as MemberSwapTarget).providerSettingsManager = { + listConfig: vi.fn().mockResolvedValue([]), + } + + await provider.handleZooCodeCallback("zoo_ext_token") + + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("[handleZooCodeCallback] Failed to save zoo-gateway profile"), + ) + // State must still be refreshed even when profile persistence fails. + expect(provider.postStateToWebview).toHaveBeenCalled() + }) it("updates every zoo-gateway profile and activates only the active one", async () => { vi.spyOn(provider, "getState").mockResolvedValue({ apiConfiguration: { zooGatewayModelId: "anthropic/claude-sonnet-4" }, - } as any) + } as never) vi.spyOn(provider.contextProxy, "getProviderSettings").mockReturnValue({ apiProvider: providerIdentifiers.zooGateway, - } as any) + } as never) vi.spyOn(provider.contextProxy, "getValues").mockReturnValue({ currentApiConfigName: "Zoo Gateway", - } as any) + } as never) const upsertSpy = vi.spyOn(provider, "upsertProviderProfile").mockResolvedValue("profile-id") const saveConfig = vi.fn().mockResolvedValue(undefined) vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi.fn().mockResolvedValue([ { name: "Zoo Gateway", apiProvider: providerIdentifiers.zooGateway }, { name: "Backup Zoo", apiProvider: providerIdentifiers.zooGateway }, @@ -8180,29 +9991,21 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }), true, ) - expect(saveConfig).toHaveBeenCalledWith( + // Both zoo-gateway profiles are refreshed - that is what every profile means - and this branch + // does it through upsertProviderProfile rather than a direct saveConfig so that a view pinned + // to a profile re-resolves it instead of serving the pre-callback config. The activation flag + // follows whether Zoo Gateway is already the active provider: this callback keeps Zoo Gateway + // active and refreshes the other profiles without activating them, so activates only the + // active one holds through the API we ship. + expect(upsertSpy).toHaveBeenCalledWith( "Backup Zoo", expect.objectContaining({ zooSessionToken: "new-token", zooGatewayBaseUrl: "https://www.zoocode.dev/api/gateway/v1", }), + false, ) - }) - - it("logs and posts state when profile persistence fails", async () => { - vi.spyOn(provider, "getState").mockRejectedValue(new Error("state unavailable")) - vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { - listConfig: vi.fn().mockResolvedValue([]), - } - - await provider.handleZooCodeCallback("zoo_ext_token") - - expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( - expect.stringContaining("[handleZooCodeCallback] Failed to save zoo-gateway profile"), - ) - // State must still be refreshed even when profile persistence fails. - expect(provider.postStateToWebview).toHaveBeenCalled() + expect(saveConfig).not.toHaveBeenCalled() }) }) @@ -8210,7 +10013,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { it("does nothing when no cached auth token exists", async () => { const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi.fn(), } @@ -8218,14 +10021,13 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { expect(handleSpy).not.toHaveBeenCalled() }) - it("skips seeding when every zoo-gateway profile already has the current token and base URL", async () => { const { getCachedZooCodeToken } = await import("../../../services/zoo-code-auth") vi.mocked(getCachedZooCodeToken).mockReturnValue("current-token") const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi .fn() .mockResolvedValue([{ name: "Zoo Gateway", apiProvider: providerIdentifiers.zooGateway }]), @@ -8235,7 +10037,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }), } - await (provider as any).ensureZooGatewayProfileSeeded() + await provider["ensureZooGatewayProfileSeeded"]() expect(handleSpy).not.toHaveBeenCalled() expect(postMessageSpy).toHaveBeenCalledWith({ type: "zooGatewayCredentialsReady" }) @@ -8246,7 +10048,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { vi.mocked(getCachedZooCodeToken).mockReturnValue("fresh-token") const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi .fn() .mockResolvedValue([{ name: "Zoo Gateway", apiProvider: providerIdentifiers.zooGateway }]), @@ -8256,7 +10058,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }), } - await (provider as any).ensureZooGatewayProfileSeeded() + await provider["ensureZooGatewayProfileSeeded"]() expect(handleSpy).toHaveBeenCalledWith("fresh-token") }) @@ -8266,7 +10068,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { vi.mocked(getCachedZooCodeToken).mockReturnValue("current-token") const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) - ;(provider as any).providerSettingsManager = { + ;(provider as MemberSwapTarget).providerSettingsManager = { listConfig: vi .fn() .mockResolvedValue([{ name: "Zoo Gateway", apiProvider: providerIdentifiers.zooGateway }]), @@ -8276,10 +10078,78 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }), } - await (provider as any).ensureZooGatewayProfileSeeded() + await provider["ensureZooGatewayProfileSeeded"]() expect(handleSpy).toHaveBeenCalledWith("current-token") }) + // The up-to-date guard reads every zoo-gateway profile rather than one end of the list, and + // it skips profiles that belong to another provider. With a single profile in the store a + // first-only, a last-only and an every-profile check all behave the same, so the order of + // the two rows below is what pins the guard down. "Other" leads the store in both rows so + // that a guard which stopped filtering by provider is caught by either row, not by one. + it.each([ + { + title: "re-seeds when a later zoo-gateway profile is stale and the first is current", + profiles: ["Other", "Zoo Gateway", "Zoo Gateway 2"], + staleProfile: "Zoo Gateway 2", + }, + { + title: "re-seeds when the first zoo-gateway profile is stale and the last is current", + profiles: ["Other", "Zoo Gateway", "Zoo Gateway 2"], + staleProfile: "Zoo Gateway", + }, + ])("$title", async ({ profiles, staleProfile }) => { + const { getCachedZooCodeToken } = await import("../../../services/zoo-code-auth") + vi.mocked(getCachedZooCodeToken).mockReturnValue("fresh-token") + const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) + const getProfile = vi.fn().mockImplementation(({ name }: { name: string }) => + // Name-keyed so the non-gateway profile reads as up to date: the only way this test + // fails on that profile is if the provider filter stops ignoring it. + name === staleProfile + ? { + zooSessionToken: "stale-token", + zooGatewayBaseUrl: "https://www.zoocode.dev/api/gateway/v1", + } + : { + zooSessionToken: "fresh-token", + zooGatewayBaseUrl: "https://www.zoocode.dev/api/gateway/v1", + }, + ) + ;(provider as MemberSwapTarget).providerSettingsManager = { + listConfig: vi.fn().mockResolvedValue( + profiles.map((name) => ({ + name, + apiProvider: + name === "Other" ? providerIdentifiers.anthropic : providerIdentifiers.zooGateway, + })), + ), + getProfile, + } + + await provider["ensureZooGatewayProfileSeeded"]() + + expect(handleSpy).toHaveBeenCalledWith("fresh-token") + expect(getProfile.mock.calls.map(([arg]) => arg.name)).not.toContain("Other") + }) + + it("re-seeds when a zoo-gateway profile carries no token at all", async () => { + const { getCachedZooCodeToken } = await import("../../../services/zoo-code-auth") + vi.mocked(getCachedZooCodeToken).mockReturnValue("fresh-token") + const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) + ;(provider as MemberSwapTarget).providerSettingsManager = { + listConfig: vi + .fn() + .mockResolvedValue([{ name: "Zoo Gateway", apiProvider: providerIdentifiers.zooGateway }]), + // An imported or half-written profile: the base URL is right, the token is absent. + getProfile: vi + .fn() + .mockResolvedValue({ zooGatewayBaseUrl: "https://www.zoocode.dev/api/gateway/v1" }), + } + + await provider["ensureZooGatewayProfileSeeded"]() + + expect(handleSpy).toHaveBeenCalledWith("fresh-token") + }) }) }) }) diff --git a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts index 47fcfd11f0..8d43ba8e5a 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts @@ -121,6 +121,8 @@ vi.mock("@roo-code/cloud", () => ({ get instance() { return { isAuthenticated: vi.fn().mockReturnValue(false), + on: vi.fn(), + off: vi.fn(), } }, }, @@ -295,9 +297,9 @@ describe("ClineProvider - Sticky Mode", () => { cspSource: "vscode-webview://test-csp-source", }, visible: true, - onDidDispose: vi.fn().mockImplementation(() => { - return { dispose: vi.fn() } - }), + // Register only: firing the callback here disposes the view before resolveWebviewView finishes, + // which sends the later stale-check down the sidebar branch. Tests that need a disposal fire it. + onDidDispose: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), onDidChangeVisibility: vi.fn().mockImplementation(() => { return { dispose: vi.fn() } }), @@ -468,6 +470,121 @@ describe("ClineProvider - Sticky Mode", () => { }) }) + describe("persisted view state keys", () => { + it("does not resolve a view id that names an Object.prototype key through the chain", async () => { + await provider.resolveWebviewView(mockWebviewView) + + // Seed through the durable write path so later writes land in the same + // backing map the assertions read: a frozen get() literal would make every + // post-write assertion read the seed no matter what the provider wrote. + await provider.contextProxy.setValue("viewStates", { "stable-test-view": { mode: "architect" } }) + + const states = provider["getPersistedViewStates"]({ fresh: true }) + expect(Object.getPrototypeOf(states)).toBeNull() + // A view id such as "constructor" must read as an absent entry, not as the + // inherited Object function, otherwise the rekey deletes the temporary entry + // and never writes it under the stable key. + expect(states["constructor"]).toBeUndefined() + expect(states["stable-test-view"]).toEqual({ mode: "architect" }) + + // Registering such an id must not make an inherited member look like a + // persisted entry: the lookup still reports an absent entry, so the rekey keeps the + // temporary entry instead of deleting it. + await provider["setViewStateId"]("constructor") + expect(provider["getPersistedViewStates"]({ fresh: true })["constructor"]).toBeUndefined() + }) + + it("does not load a temporary-id entry that an earlier session wrote", async () => { + // Temporary names restart from 0 on every host start, so storage can still hold + // an entry under this instance's name from a previous session. Seed the entry + // through the ContextProxy before the provider is constructed so the ordinary + // loadViewState read (which goes through the proxy cache) actually sees it. + const tempId = `sidebar-${ClineProvider["nextViewId"]}` + const proxy = new ContextProxy(mockContext) + await proxy.setValue("viewStates", { + [tempId]: { mode: "architect", currentApiConfigName: "stale-profile" }, + }) + + const fresh = new ClineProvider(mockContext, mockOutputChannel, "sidebar", proxy, new WebviewFocusTracker()) + expect(fresh.viewId).toBe(tempId) + + // Await the load itself instead of a fixed timer: the negative + // assertion must run after the load has definitely finished. + await fresh["loadViewState"]() + + // The unowned temporary-id entry must not be adopted... + expect(fresh["viewLocalState"]).toStrictEqual({}) + + // ...and the shared values still win, so the stale entry cannot reach getState(). + const state = await fresh.getState() + expect(state.mode).toBe("code") + expect(state.currentApiConfigName).not.toBe("stale-profile") + }) + + it("does not merge a stale temporary-id entry into this instance's first durable write", async () => { + // A first write under an unowned temporary id claims the slot: fields left by an + // earlier session under the same name must not be adopted (and later re-keyed to + // the stable id) by merging into the colliding entry. + const tempId = `sidebar-${ClineProvider["nextViewId"]}` + const proxy = new ContextProxy(mockContext) + await proxy.setValue("viewStates", { [tempId]: { currentApiConfigName: "old-profile", updatedAt: 1 } }) + + const fresh = new ClineProvider(mockContext, mockOutputChannel, "sidebar", proxy, new WebviewFocusTracker()) + expect(fresh.viewId).toBe(tempId) + + await fresh.saveViewState("mode", "code") + + const states = fresh["getPersistedViewStates"]({ fresh: true }) + expect(states[tempId]).toEqual({ + mode: "code", + updatedAt: expect.any(Number), + claimToken: expect.any(String), + }) + expect(fresh["viewLocalState"].currentApiConfigName).toBeUndefined() + }) + + it("does not mark the temporary id as authored when the durable write fails", async () => { + // Authorship must be recorded only after the write lands: a failed persistence + // that still marked the id owned would let a later load adopt an entry that was + // never actually stored. + const tempId = `sidebar-${ClineProvider["nextViewId"]}` + const proxy = new ContextProxy(mockContext) + const fresh = new ClineProvider(mockContext, mockOutputChannel, "sidebar", proxy, new WebviewFocusTracker()) + expect(fresh.viewId).toBe(tempId) + + const originalUpdate = mockContext.globalState.update + mockContext.globalState.update = vi.fn().mockImplementation((key: string, _value: unknown) => { + if (key === "viewStates") { + return Promise.reject(new Error("storage unavailable")) + } + return Promise.resolve() + }) + + await expect(fresh.saveViewState("mode", "code")).rejects.toThrow("storage unavailable") + expect(fresh["wroteUnderTemporaryViewStateId"]).toBe(false) + + mockContext.globalState.update = originalUpdate + }) + + it("does not adopt an entry under the temporary name that this instance never wrote", async () => { + await provider.resolveWebviewView(mockWebviewView) + + // Temporary ids restart from 0 on every host start, so the same name can + // belong to a different session. An entry this instance never wrote must not + // be adopted as this view's state. + // Seed through the durable write path so the assertions below read the + // same backing map a re-key adoption would write to: a frozen get() + // literal made the negative assertion unable to fail. + await provider.contextProxy.setValue("viewStates", { [provider.viewId]: { mode: "architect" } }) + + await provider["setViewStateId"]("stable-test-view") + + const states = provider["getPersistedViewStates"]({ fresh: true }) + expect(states[provider.viewId]).toEqual({ mode: "architect" }) + expect(states["stable-test-view"]).toBeUndefined() + }) + }) + describe("createTaskWithHistoryItem", () => { it("should restore mode from history item when reopening task", async () => { await provider.resolveWebviewView(mockWebviewView) @@ -486,14 +603,15 @@ describe("ClineProvider - Sticky Mode", () => { mode: "architect", // Saved mode } - // Mock updateGlobalState to track mode updates - const updateGlobalStateSpy = vi.spyOn(provider as any, "updateGlobalState").mockResolvedValue(undefined) + // Register a stable view id so the durable per-view write is persisted + await provider["setViewStateId"]("stable-test-view") // Initialize task with history item await provider.createTaskWithHistoryItem(historyItem) - // Verify mode was restored via updateGlobalState - expect(updateGlobalStateSpy).toHaveBeenCalledWith("mode", "architect") + // Verify mode was restored into the view-local pin (no shared global write) + expect(provider["viewLocalState"].mode).toBe("architect") + expect(mockContext.globalState.update).not.toHaveBeenCalledWith("mode", "architect") }) it("should use current mode if history item has no saved mode", async () => { @@ -770,13 +888,20 @@ describe("ClineProvider - Sticky Mode", () => { // Restore the task from history await provider.createTaskWithHistoryItem(historyItem) - // Verify that the mode was restored + // Verify that the mode was restored into this view's durable pin. The + // getState() merge of hydrated per-view values lands with the F1b follow-up. + expect(provider["viewLocalState"].mode).toBe("architect") + const state = await provider.getState() - expect(state.mode).toBe("architect") // Verify that the API configuration was also restored expect(state.currentApiConfigName).toBe("architect-config") expect(state.apiConfiguration.apiProvider).toBe("openai") + // The restored mode lives only in this view's buffer, so the restore must not + // re-bind the shared mode slot. Before the fix this wrote architect-config onto + // the "code" slot, because getState().mode still reported code. + expect(await provider.providerSettingsManager.getModeConfigId("code")).toBe(codeConfigId) + expect(await provider.providerSettingsManager.getModeConfigId("architect")).toBe(architectConfigId) }) it("should handle mode deletion between sessions", async () => { @@ -1275,4 +1400,37 @@ describe("ClineProvider - Sticky Mode", () => { }) }) }) + + describe("setViewStateId input validation", () => { + it("ignores a non-string viewStateId from a crafted message", async () => { + const before = provider["viewStateId"] + // A crafted postMessage can carry any JSON value despite the declared type. + await provider.setViewStateId(42 as unknown as string) + expect(provider["viewStateId"]).toBe(before) + }) + + it("ignores an oversized viewStateId", async () => { + const before = provider["viewStateId"] + await provider.setViewStateId("x".repeat(200)) + expect(provider["viewStateId"]).toBe(before) + }) + + it("refuses a viewStateId already claimed by another live view", async () => { + const other = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + try { + await other.setViewStateId("claimed-by-other-view") + await provider.setViewStateId("claimed-by-other-view") + expect(provider["viewStateId"]).not.toBe("claimed-by-other-view") + expect(other["viewStateId"]).toBe("claimed-by-other-view") + } finally { + await other.dispose() + } + }) + }) }) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 42be14463c..df8d17de75 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -921,12 +921,12 @@ }, "core/webview/__tests__/ClineProvider.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 195 + "count": 151 } }, "core/webview/__tests__/ClineProvider.sticky-mode.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 27 + "count": 26 } }, "core/webview/__tests__/ClineProvider.sticky-profile.spec.ts": {