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..f8d10084e8 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -113,7 +113,14 @@ export interface ExtensionMessage { | "rooHistoryImportProgress" | "themeFixtureProbeRequest" | "diagnosticsRequest" + | "viewStateClaim" text?: string + /** + * For `viewStateClaim`: the durable `viewStates` key the delivered claim governs. + */ + viewStateId?: string + /** For `viewStateClaim`: the host-minted token the webview stores and echoes on launch. */ + viewStateClaim?: string /** For fileContent: { path, content, error? } */ fileContent?: { path: string; content: string | null; error?: string } /** For originalContent: the pre-edit file content of the requested tool message (null when unavailable) */ diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index e53f60fb83..64023b9b9c 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -63,18 +63,35 @@ const withinReadBackDeadline = (read: PromiseLike, key: string): Promise PASS_THROUGH_STATE_KEYS.includes(key) +// A parsed payload can carry these as own keys. Writing one through a computed assignment +// re-points the target object's prototype instead of storing a setting, so they are never +// accepted as state keys. +const UNSAFE_STATE_KEYS = ["__proto__", "constructor", "prototype"] + +export const isUnsafeStateKey = (key: string) => UNSAFE_STATE_KEYS.includes(key) + 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 + // A reset that is already running. Overlapping callers join it instead of starting a second + // delete pass whose end could lower the guard while the first one is still writing. + private resetInFlight: Promise | null = null constructor(context: vscode.ExtensionContext) { this.originalContext = context @@ -401,6 +418,10 @@ export class ContextProxy { private readonly secretWriteCount = new Map() updateGlobalState(key: K, value: GlobalState[K]) { + if (isUnsafeStateKey(key)) { + return Promise.resolve() + } + if (isPassThroughStateKey(key)) { return this.originalContext.globalState.update(key, value) } @@ -413,6 +434,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 +452,32 @@ export class ContextProxy { }) } + /** + * The write generation the next `setValue`/`setValues` of this key will produce, or + * `undefined` when the key's write path records no generation at all - a secret, or a + * pass-through key that bypasses the state cache. Capture it before awaiting the write. + */ + public beginStateWriteGeneration(key: K): number | undefined { + if (!this.recordsWriteGeneration(key)) { + return undefined + } + + return (this.stateWriteGenerations.get(key as GlobalStateKey) ?? 0) + 1 + } + + /** + * Current write generation for a key, or `undefined` when its write path records none. A + * caller that may have to undo its own write compares it against the value captured from + * `beginStateWriteGeneration`: a newer write for the same key has already advanced it. + */ + public getStateWriteGeneration(key: K): number | undefined { + return this.recordsWriteGeneration(key) ? this.stateWriteGenerations.get(key as GlobalStateKey) : undefined + } + + private recordsWriteGeneration(key: K): boolean { + return !isSecretStateKey(key) && !isPassThroughStateKey(key) + } + private getAllGlobalState(): GlobalState { return Object.fromEntries(GLOBAL_STATE_KEYS.map((key) => [key, this.getGlobalState(key)])) } @@ -443,6 +492,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) @@ -715,18 +771,46 @@ export class ContextProxy { * This clears all data from both the in-memory caches and the VSCode storage. * @returns A promise that resolves when all reset operations are complete */ - public async resetAllState() { - // Clear in-memory caches - this.stateCache = {} - this.secretCache = {} + public resetAllState(): Promise { + // Two resets can overlap (a second webview, or the same command twice). A boolean guard lets the + // first pass to finish lower it while the second is still deleting, and a secret stored in that + // gap is erased by the pass that is still running - so overlapping callers join the running pass. + if (this.resetInFlight) { + return this.resetInFlight + } - 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)), - ]) + this.resetting = true + const pass = (async () => { + try { + 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 this.initialize() + // Promise.all rejects on the first failure while the other updates and deletes are still + // pending, and the finally below would then admit a storeSecret that a still-running + // delete erases. Settle every delete first, then surface the first failure. + const deletions = await Promise.allSettled([ + ...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)), + ]) + const deletionFailure = deletions.find((result) => result.status === "rejected") + if (deletionFailure) { + throw deletionFailure.reason + } + + await this.initialize() + } finally { + this.resetting = false + this.resetInFlight = null + } + })() + this.resetInFlight = pass + return pass } private static _instance: ContextProxy | null = null diff --git a/src/core/config/CustomModesManager.ts b/src/core/config/CustomModesManager.ts index 86bab479d7..8728696bf7 100644 --- a/src/core/config/CustomModesManager.ts +++ b/src/core/config/CustomModesManager.ts @@ -353,6 +353,28 @@ export class CustomModesManager { } } + /** + * Every definition of a mode, per source file, without the project-over-global hiding that + * `getCustomModes` applies. A rollback has to restore both: a batch that overwrites a global + * definition through a slug that also exists as a project mode would otherwise leave the global + * copy permanently replaced by the batch's version. + */ + public async getCustomModeDefinitions(slugs: readonly string[]): Promise { + const wanted = new Set(slugs) + const settingsPath = await this.getCustomModesFilePath() + const settingsModes = await this.loadModesFromFile(settingsPath) + const roomodesPath = await this.getWorkspaceRoomodes() + const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : [] + + return [ + ...roomodesModes + .filter((mode) => wanted.has(mode.slug)) + .map((mode) => ({ ...mode, source: "project" as const })), + ...settingsModes + .filter((mode) => wanted.has(mode.slug)) + .map((mode) => ({ ...mode, source: "global" as const })), + ] + } public async getCustomModes(): Promise { // Check if we have a valid cached result. const now = Date.now() @@ -553,6 +575,44 @@ export class CustomModesManager { } } + /** + * Removes a mode's entry from the mode files and refreshes the merged state, without + * touching its rules folder. Rollback paths need this variant: `deleteCustomMode` also + * deletes `.roo/rules-` (or `~/.roo/rules-`), and a rollback cannot know + * whether that folder predates the batch whose registration it is undoing - deleting it + * would discard rules the user wrote earlier. + * + * Returns whether an entry was actually removed so a caller can report a miss instead of + * assuming cleanup happened. Errors propagate; rollback callers decide how loud to be. + */ + public async removeCustomModeEntry(slug: string): Promise { + const settingsPath = await this.getCustomModesFilePath() + const roomodesPath = await this.getWorkspaceRoomodes() + const projectModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : [] + const settingsModes = await this.loadModesFromFile(settingsPath) + const inProjectFile = projectModes.some((mode) => mode.slug === slug) + const inSettingsFile = settingsModes.some((mode) => mode.slug === slug) + + if (!inProjectFile && !inSettingsFile) { + return false + } + + await this.queueWrite(async () => { + if (roomodesPath && inProjectFile) { + await this.updateModesInFile(roomodesPath, (modes) => modes.filter((m) => m.slug !== slug)) + } + + if (inSettingsFile) { + await this.updateModesInFile(settingsPath, (modes) => modes.filter((m) => m.slug !== slug)) + } + + this.clearCache() + await this.refreshMergedState() + }) + + return true + } + /** * Deletes the rules folder for a specific mode * @param slug - The mode slug 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..468965a0b8 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -2,6 +2,8 @@ import * as vscode from "vscode" +import type { RooCodeSettings } from "@roo-code/types" + import { GLOBAL_STATE_KEYS, SECRET_STATE_KEYS, GLOBAL_SECRET_KEYS } from "@roo-code/types" import { clearAllMocks } from "../../../test-utils/reset" @@ -121,6 +123,115 @@ 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 bypass cache for pass-through state keys", async () => { // Setup mock return value mockGlobalState.get.mockReturnValue("pass-through-value") @@ -1170,4 +1281,71 @@ Output only the summary of the conversation so far, without any additional comme expect(customSupportPromptsUpdateCalls.length).toBe(0) }) }) + + describe("payload-shaped state keys", () => { + it("should refuse a prototype key carried by a parsed payload", async () => { + // JSON.parse gives an object an own "__proto__" key, and `setValues` iterates own keys. + // Accepting it would re-point the state cache's prototype instead of storing a setting, + // and every later read of a real key could then resolve through that prototype. + const payload = JSON.parse('{"__proto__":{"polluted":true},"mode":"code"}') as RooCodeSettings + + await proxy.setValues(payload) + + expect(proxy.getValue("mode")).toBe("code") + const stateCache = proxy["stateCache"] as Record + expect(Object.getPrototypeOf(stateCache)).toBe(Object.prototype) + expect(stateCache["polluted"]).toBeUndefined() + expect(({} as Record).polluted).toBeUndefined() + expect(mockGlobalState.update.mock.calls.some((call: unknown[]) => call[0] === "__proto__")).toBe(false) + }) + + it("should refuse the same key on the global-state write path", async () => { + // `updateGlobalState` is the function that performs the computed assignment, so it + // rejects the key itself rather than relying on its callers. + await proxy.updateGlobalState("__proto__" as never, { polluted: true } as never) + await proxy.updateGlobalState("constructor" as never, { polluted: true } as never) + + const stateCache = proxy["stateCache"] as Record + expect(Object.getPrototypeOf(stateCache)).toBe(Object.prototype) + expect(stateCache["polluted"]).toBeUndefined() + expect(mockGlobalState.update).not.toHaveBeenCalledWith("__proto__", expect.anything()) + }) + }) + + describe("state write generations", () => { + it("should report the generation the next global-state write produces", async () => { + expect(proxy.beginStateWriteGeneration("writeDelayMs")).toBe(1) + + await proxy.setValue("writeDelayMs", 10) + + expect(proxy.getStateWriteGeneration("writeDelayMs")).toBe(1) + expect(proxy.beginStateWriteGeneration("writeDelayMs")).toBe(2) + }) + + it("should record no generation for keys that bypass the state cache", () => { + // Secrets live in ExtensionContext.secrets and `taskHistory` is written straight + // through to global state. Neither path advances a generation, so a rollback of such a + // write keeps the value-only check instead of claiming a token it cannot honour. + expect(proxy.beginStateWriteGeneration("apiKey")).toBeUndefined() + expect(proxy.getStateWriteGeneration("apiKey")).toBeUndefined() + expect(proxy.beginStateWriteGeneration("taskHistory")).toBeUndefined() + expect(proxy.getStateWriteGeneration("taskHistory")).toBeUndefined() + }) + }) + + 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__/CustomModesManager.spec.ts b/src/core/config/__tests__/CustomModesManager.spec.ts index eddaec5621..4e9c797f54 100644 --- a/src/core/config/__tests__/CustomModesManager.spec.ts +++ b/src/core/config/__tests__/CustomModesManager.spec.ts @@ -764,6 +764,70 @@ describe("CustomModesManager", () => { }) }) + describe("removeCustomModeEntry", () => { + const wireSettingsFile = (initialModes: ModeConfig[]) => { + let settingsContent = { customModes: initialModes } + + ;(fs.readFile as Mock).mockImplementation(async (path: string) => { + if (path === mockSettingsPath) { + return yaml.stringify(settingsContent) + } + throw new Error("File not found") + }) + ;(fs.writeFile as Mock).mockImplementation(async (path: string, content: string) => { + if (path === mockSettingsPath) { + settingsContent = yaml.parse(content) + } + return Promise.resolve() + }) + ;(mockContext.globalState.update as Mock).mockImplementation((key: string, value: ModeConfig[]) => { + if (key === "customModes") { + settingsContent.customModes = value + } + return Promise.resolve() + }) + + return () => settingsContent.customModes + } + + it("removes the mode entry without deleting the mode's rules folder", async () => { + // The mode already had a rules folder on disk before this batch registered it - exactly + // the case a rollback has to survive. + ;(fileExistsAtPath as Mock).mockImplementation( + async (p: string) => p === mockSettingsPath || p === mockRoomodes || p.includes("rules-batch-mode"), + ) + const batchMode = { + slug: "batch-mode", + name: "Batch Mode", + roleDefinition: "Test role", + groups: ["read" as const], + source: "global" as const, + } + const readModes = wireSettingsFile([batchMode]) + + const removed = await manager.removeCustomModeEntry("batch-mode") + + expect(removed).toBe(true) + expect(readModes()).toHaveLength(0) + expect(mockContext.globalState.update).toHaveBeenCalledWith("customModes", []) + + // The discriminating assertion: `deleteCustomMode` reaches `fs.rm` for the rules folder. + // A rollback of a registration this process made must not delete rules the user wrote + // before it, so this path never touches them. + expect(fs.rm).not.toHaveBeenCalled() + }) + + it("reports a miss when no mode file holds the slug", async () => { + const readModes = wireSettingsFile([ + { slug: "keep-me", name: "Keep", roleDefinition: "r", groups: ["read" as const] }, + ]) + + expect(await manager.removeCustomModeEntry("absent-mode")).toBe(false) + expect(fs.writeFile).not.toHaveBeenCalled() + expect(readModes()).toHaveLength(1) + }) + }) + describe("updateModesInFile", () => { it("handles corrupted YAML content gracefully", async () => { const corruptedYaml = "customModes: [invalid yaml content" 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 87c37cf596..8e3b9b884a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -74,6 +74,7 @@ import { isRetiredProvider, modelIdKeys, providerIdentifiers, + PROVIDER_SETTINGS_KEYS, } from "@roo-code/types" import { RateLimitClock, createRateLimitClock } from "../task/RateLimitClock" import { TaskRegistry } from "../task/TaskRegistry" @@ -160,6 +161,10 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" +type PersistedViewState = NonNullable[string] +type ViewLocalStateValues = Partial & Partial +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 +253,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 +424,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 = "oct-2026-v3.94.0-models-oauth-chat" // v3.94.0 model support, MCP OAuth, and chat reliability @@ -436,6 +481,9 @@ export class ClineProvider (message) => this.log(message), ) + // Derive the per-instance view id and bind the persisted view-state key to it. + this.viewId = `${renderContext}-${ClineProvider.nextViewId++}` + this.viewStateId = this.viewId ClineProvider.activeInstances.add(this) this.diagnosticsRecorder.record({ boundary: "provider", phase: "success", type: "created" }) @@ -599,6 +647,427 @@ 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 { + return Object.fromEntries( + Object.entries(states) + .sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)) + .slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES), + ) + } + + /** 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) { + loadedState.currentApiConfigName = persisted.currentApiConfigName + + try { + const { name: _name, ...apiConfiguration } = await this.providerSettingsManager.getProfile({ + name: persisted.currentApiConfigName, + }) + loadedState.apiConfiguration = apiConfiguration as ProviderSettings + } catch (error) { + this.log( + `[loadViewState] Unable to resolve API profile '${persisted.currentApiConfigName}' for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + + if (this.viewStateId !== loadedForViewId) { + this.log(`[loadViewState] Discarding stale state for superseded view id ${loadedForViewId}`) + return + } + + // Diff each field so in-flight edits win over the loaded values (including clears) + // while untouched fields keep the persisted values authoritative. + const postLoadBuffer = this.viewLocalState + const mergedState: Partial = { ...loadedState } + + if (!Object.is(preLoadBuffer.mode, postLoadBuffer.mode)) { + if (postLoadBuffer.mode === undefined) { + delete mergedState.mode + } else { + mergedState.mode = postLoadBuffer.mode + } + } + + if (!Object.is(preLoadBuffer.currentApiConfigName, postLoadBuffer.currentApiConfigName)) { + if (postLoadBuffer.currentApiConfigName === undefined) { + delete mergedState.currentApiConfigName + } else { + mergedState.currentApiConfigName = postLoadBuffer.currentApiConfigName + } + } + + if (!Object.is(preLoadBuffer.apiConfiguration, postLoadBuffer.apiConfiguration)) { + if (postLoadBuffer.apiConfiguration === undefined) { + delete mergedState.apiConfiguration + } else { + mergedState.apiConfiguration = postLoadBuffer.apiConfiguration + } + } + + // 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 */ @@ -2368,6 +2837,8 @@ export class ClineProvider if (signal.aborted) return id + 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 +2847,18 @@ 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 every view + // that buffered a copy (pinned siblings included), or their getState() keeps the + // pre-save values. + if (this.pinnedProfileName === name) { + await this._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }) + // Only rebuild a task that actually runs under the saved profile. + if (this.getCurrentTask()?.taskApiConfigName === name) { + this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true }) + } + } + await this.refreshSiblingsAfterProfileSave(name, providerSettings) } if (signal.aborted) return id @@ -2383,6 +2866,32 @@ export class ClineProvider await this.postStateToWebview() return id } + /** + * Refresh the buffered overlay of every live sibling pinned to a saved profile and rebuild + * any task that follows it. The handler rebuild is independent of the pin: a task restored + * from history can follow profile A while its view pins profile B. + */ + private async refreshSiblingsAfterProfileSave(name: string, providerSettings: ProviderSettings): Promise { + for (const instance of ClineProvider.getAllInstances()) { + if (instance === this || instance._disposed || instance.pinnedProfileName !== name) { + continue + } + // Direct private access: compile-time safe across sibling instances. + await instance._saveViewLocalStateFromMutation({ apiConfiguration: providerSettings }) + } + + const { + name: _recordName, + id: _recordId, + ...taskSettings + } = providerSettings as ProviderSettings & { name?: string; id?: string } + for (const instance of ClineProvider.getAllInstances()) { + if (instance._disposed || instance.getCurrentTask()?.taskApiConfigName !== name) { + continue + } + instance.updateTaskApiHandlerIfNeeded(taskSettings, { forceRebuild: true }) + } + } async deleteProviderProfile(profileToDelete: ProviderSettingsEntry) { const globalSettings = this.contextProxy.getValues() @@ -3707,7 +4216,126 @@ export class ClineProvider } public async setValue(key: K, value: RooCodeSettings[K]) { + const previousSharedValues = { [key]: this.contextProxy.getValue(key) } as RooCodeSettings + // Sampled before the write is awaited: this write produces the next generation for the + // key, and a write that starts during the await produces the one after it. A key whose + // write path records no generation yields undefined and keeps the value-only check. + const writeGeneration = this.contextProxy.beginStateWriteGeneration(key as never) await this.contextProxy.setValue(key, value) + try { + await this._saveViewLocalStateFromMutation({ [key]: value } as ViewLocalStateValues) + } catch (error) { + await this._restoreSharedValuesAfterFailedViewWrite( + previousSharedValues, + { + [key]: value, + } as RooCodeSettings, + { [key]: writeGeneration }, + ) + throw error + } + } + + /** + * 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. + * + * `generations` carries the generation this attempt produced for each key it wrote. It is + * optional: a caller writing through a path that records no generation rolls back on the + * value check alone. + */ + private async _restoreSharedValuesAfterFailedViewWrite( + previous: RooCodeSettings, + attempted: RooCodeSettings, + generations: Record = {}, + ): Promise { + // Roll back only the keys this attempt wrote, that still hold the value it wrote, and + // whose write generation is still the one this attempt produced. The value check alone is + // not enough: another provider can write the *same* value while this attempt's per-view + // save is in flight, and restoring the captured previous value would silently undo that + // newer write for every view that reads shared state. + const rollback: RooCodeSettings = {} as RooCodeSettings + + for (const keyName of Object.keys(attempted)) { + // `attempted` can originate in a parsed payload, and a parsed payload can carry + // "__proto__". Assigning that through a computed key would modify Object.prototype + // instead of recording a rollback entry. + if (keyName === "__proto__" || keyName === "constructor" || keyName === "prototype") { + continue + } + + const key = keyName as keyof RooCodeSettings + const generation = generations[keyName] + const ownsThisWrite = + generation === undefined || this.contextProxy.getStateWriteGeneration(key) === generation + + if (ownsThisWrite && this.contextProxy.getValue(key) === attempted[key]) { + ;(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)}`, + ) + } } public getValue(key: K) { @@ -3719,9 +4347,80 @@ export class ClineProvider } 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 + // Per-key generations sampled before the batch is awaited: the batch produces the next + // generation for every key it writes, so a key another view writes - during the batch or + // during the per-view save below - ends up holding a later one. + const writeGenerations = Object.fromEntries( + Object.keys(sanitizedValues).map((key) => [key, this.contextProxy.beginStateWriteGeneration(key as never)]), + ) as Record + await this.contextProxy.setValues(sanitizedValues) + 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. Only keys this + // batch still owns are restored. + 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 { + // Captured here, not in the setters: saveViewState() and the profile paths call this + // helper directly, so a reset during their durable write must skip the buffer half too. + const generation = this.viewLocalStateGeneration + await this._persistViewLocalStateFromMutation(values) + 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 @@ -3750,9 +4449,32 @@ export class ClineProvider } await this.contextProxy.resetAllState() + + // Clear every live view's buffer, not just this view's: a sibling's getConfiguration() + // would otherwise keep serving pre-reset selections. + for (const instance of ClineProvider.getAllInstances()) { + if (!instance._disposed) { + instance._clearViewLocalState() + } + } + await this.providerSettingsManager.resetAllConfigs() await this.customModesManager.resetCustomModes() await this.removeClineFromStack() + + // Clear this view's persisted entry last, and tolerate a storage failure: the shared + // stores, custom modes and the running task are already reset, so a viewStates write + // error must not abort the reset half-way or skip the state post below. + try { + // Serialize with profile saves: a save queued during the reset must not write this + // view's entry after it is cleared. + await this.enqueueProviderProfileMutation(() => this.clearPersistedViewState()) + } catch (error) { + this.log( + `[resetState] Failed to clear the persisted view state: ${error instanceof Error ? error.message : String(error)}`, + ) + } + await this.postStateToWebview() await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) } @@ -4278,42 +5000,87 @@ export class ClineProvider configuration: RooCodeSettings = {}, ): Promise { if (configuration) { - await this.setValues(configuration) + // Capture the modes this batch overrides before registering them: a later failure + // must restore a previously installed mode instead of leaving the batch's copy. + // Snapshot every definition of every slug this batch touches, one per source file. Filtering + // the merged list would keep only the project copy of a slug that exists in both files, and + // the rollback would then restore that copy while the global definition stayed overwritten. + const savedCustomModes = configuration.customModes?.length + ? await this.customModesManager.getCustomModeDefinitions( + configuration.customModes.map((candidate) => candidate.slug), + ) + : [] - if (configuration.allowedCommands) { - await vscode.workspace - .getConfiguration(Package.name) - .update("allowedCommands", configuration.allowedCommands, vscode.ConfigurationTarget.Global) - } + // Registration lives inside the try below: a mode whose write fails partway through + // the batch must trigger the same rollback as a failure after setValues. - if (configuration.deniedCommands) { - await vscode.workspace - .getConfiguration(Package.name) - .update("deniedCommands", configuration.deniedCommands, vscode.ConfigurationTarget.Global) - } + try { + // Register the batch's custom modes before setValues validates `mode`: a mode + // selected in the same configuration is not known to the manager yet. + if (configuration.customModes?.length) { + for (const mode of configuration.customModes) { + await this.customModesManager.updateCustomMode(mode.slug, mode) + } + } + await this.setValues(configuration) - if (configuration.commandExecutionTimeout !== undefined) { - await vscode.workspace - .getConfiguration(Package.name) - .update( - "commandExecutionTimeout", - configuration.commandExecutionTimeout, - vscode.ConfigurationTarget.Global, - ) - } + if (configuration.allowedCommands) { + await vscode.workspace + .getConfiguration(Package.name) + .update("allowedCommands", configuration.allowedCommands, vscode.ConfigurationTarget.Global) + } - if (configuration.currentApiConfigName) { - await this.setProviderProfile(configuration.currentApiConfigName) - } + if (configuration.deniedCommands) { + await vscode.workspace + .getConfiguration(Package.name) + .update("deniedCommands", configuration.deniedCommands, vscode.ConfigurationTarget.Global) + } + + if (configuration.commandExecutionTimeout !== undefined) { + await vscode.workspace + .getConfiguration(Package.name) + .update( + "commandExecutionTimeout", + configuration.commandExecutionTimeout, + vscode.ConfigurationTarget.Global, + ) + } + + if (configuration.currentApiConfigName) { + await this.setProviderProfile(configuration.currentApiConfigName) + } + } catch (error) { + // A failure after the registration must not leave the batch's modes installed: + // a slug that already existed is restored to its earlier definition, and a slug this + // batch introduced has no earlier definition to restore. + for (const savedMode of savedCustomModes) { + await this.customModesManager.updateCustomMode(savedMode.slug, savedMode).catch((restoreError) => { + this.log( + `[createTask] Failed to restore custom mode '${savedMode.slug}' after the task creation failed: ${ + restoreError instanceof Error ? restoreError.message : String(restoreError) + }`, + ) + }) + } + + // Removal uses `removeCustomModeEntry`, not `deleteCustomMode`: the public delete also + // removes `.roo/rules-` (`~/.roo/rules-` for a global mode) without knowing + // whether that folder predates this batch, and a rollback must not destroy rules the + // user wrote earlier. + for (const mode of configuration.customModes ?? []) { + if (savedCustomModes.some((saved) => saved.slug === mode.slug)) { + continue + } - // 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) + await this.customModesManager.removeCustomModeEntry(mode.slug).catch((removeError) => { + this.log( + `[createTask] Failed to remove the custom mode '${mode.slug}' registered by the failed task creation: ${ + removeError instanceof Error ? removeError.message : String(removeError) + }`, + ) + }) } + throw error } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 9826fc206b..7a2df8d97a 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -30,10 +30,12 @@ import { defaultModeSlug } from "../../../shared/modes" import { experimentDefault } from "../../../shared/experiments" import { setTtsEnabled } from "../../../utils/tts" import { ContextProxy } from "../../config/ContextProxy" +import type { ProviderSettingsManager } from "../../config/ProviderSettingsManager" import { WorkspaceIndexingEnablementManager } from "../../../services/code-index/workspace-indexing-enablement-manager" import { Task, TaskOptions } from "../../task/Task" import { safeWriteJson } from "../../../utils/safeWriteJson" +import { t } from "../../../i18n" import { ClineProvider } from "../ClineProvider" import { webviewMessageHandler } from "../webviewMessageHandler" import { Terminal } from "../../../integrations/terminal/Terminal" @@ -425,6 +427,38 @@ afterAll(() => { vi.restoreAllMocks() }) +// The stalled profile is typed from the manager's own return type so the profile spies below need no +// assertion, and the shape stays what a real getProfile resolves to (id optional, so the doubles that omit +// it keep matching what the load path receives). +type StalledProfile = Awaited> + +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)) + }) + // The manager field is readonly and getProfile is the only member this scenario reaches, so the spy goes on the + // method rather than onto a replacement manager: the double stays checked against the real signature, the + // one-call guard below still fails loudly on a second lookup, and no suppression directive is needed. + vi.spyOn(provider.providerSettingsManager, "getProfile").mockImplementation(() => 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) { @@ -5719,6 +5753,1357 @@ describe("ClineProvider", () => { ]) }) }) + + describe("setViewStateId", () => { + it('should ignore "__proto__" and keep the temporary viewId', async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["setViewStateId"]("__proto__") + + // "__proto__" is rejected before assignment so a per-view entry can never be + // keyed through the Object.prototype setter: the temporary id stays active and + // nothing is persisted under the reserved name. + expect(provider["viewStateId"]).toBe(provider.viewId) + expect(mockContext.globalState.get("viewStates")).toBeUndefined() + expect(provider["viewLocalState"]).toEqual({}) + + await provider.dispose() + }) + + it("restores the previous view id when the registration write fails so a later launch retries", async () => { + const contextProxy = new ContextProxy(mockContext) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + // Seed a pre-launch entry under the temporary id through the real write path, + // so the re-key has real work to do and records this instance as its author. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + + const setValueSpy = vi.spyOn(contextProxy, "setValue").mockRejectedValue(new Error("storage down")) + + await expect(provider["setViewStateId"]("stable-sidebar-view")).rejects.toThrow("storage down") + + // The failed id must not stick: the provider keeps its previous (temporary) + // id so a later launch retries registration and the load instead of the + // guard early-returning for an id that was never persisted. + expect(provider["viewStateId"]).toBe(provider.viewId) + + // A later retry succeeds once the storage write works again, and the + // pre-launch entry lands under the registered id. + setValueSpy.mockRestore() + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewStateId"]).toBe("stable-sidebar-view") + expect(mockContext.globalState.get("viewStates")).toEqual({ + "stable-sidebar-view": { mode: "architect", updatedAt: 1, claimToken: expect.any(String) }, + }) + + await provider.dispose() + }) + }) + + describe("view state claim binding", () => { + it("refuses a viewStateId bound by a concurrent registration through setViewStateId", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const beforeViewStateId = provider["viewStateId"] + const postSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + // A claim-less entry is adoptable at the first read, but a concurrent registration + // binds the key while this launch's queued re-bind is waiting its turn. + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + await provider["setViewStateId"]("legacy-view") + + // The launch must not adopt a key another webview now owns: no claim is delivered, + // the provider stays on its temporary id, and the winner's claim is untouched. + expect(postSpy).not.toHaveBeenCalledWith(expect.objectContaining({ type: "viewStateClaim" })) + expect(provider["viewStateId"]).toBe(beforeViewStateId) + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + postSpy.mockRestore() + await provider.dispose() + }) + + it("does not overwrite a claim bound by a concurrent registration", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + // A concurrent registration binds this key while the re-bind write is queued. + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + const claim = await provider["rebindUnboundViewStateEntry"]("legacy-view") + + // The queued re-bind must not replace the claim the other registration wrote; it + // reports that nothing was rebound so the caller can refuse the stale adoption. + expect(claim).toBeUndefined() + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + await provider.dispose() + }) + + const createProvider = () => + new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const storedEntries = () => + mockContext.globalState.get>("viewStates") ?? {} + + test("refuses a disposed view's id when no claim is presented, and loads nothing", async () => { + // The vulnerable path: a view registers a durable entry, is disposed, and a second + // webview presents the same id. dispose() removes the instance but leaves the entry, + // so only the claim can stop the second webview from loading the first one's pinned + // profile (and the profile's secrets) through the state post. + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("mode", "architect") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + + const storedClaim = storedEntries()["closed-view"]?.claimToken + expect(storedClaim).toEqual(expect.any(String)) + + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + await intruder["setViewStateId"]("closed-view") + + // Fail closed: the id is not adopted, no selections reach this view and no state is + // posted back to the webview that asked for it. + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect((await intruder.getState()).currentApiConfigName).not.toBe("profile-with-secrets") + expect(postMessageSpy).not.toHaveBeenCalled() + + // A refused adoption is side-effect free: presenting an id must not probe or rewrite + // the entry that a different webview owns. + expect(storedEntries()["closed-view"]).toMatchObject({ + mode: "architect", + currentApiConfigName: "profile-with-secrets", + claimToken: storedClaim, + }) + + await intruder.dispose() + }) + + test("refuses a mismatched claim", async () => { + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + // A webview-generated value is not a claim: only the token the host minted matches. + await intruder["setViewStateId"]("closed-view", "webview-chosen-claim") + + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect(postMessageSpy).not.toHaveBeenCalled() + + await intruder.dispose() + }) + + test("does not churn the claim on later writes to the same entry", async () => { + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + await provider["setViewStateId"]("minted-view") + await provider.saveViewState("mode", "architect") + + const mintedClaim = storedEntries()["minted-view"]?.claimToken + expect(mintedClaim).toEqual(expect.any(String)) + + await provider.saveViewState("currentApiConfigName", "profile-b") + + // A merge into an entry this view already owns keeps its token, so the claim the + // webview holds keeps working and is not re-sent. + expect(storedEntries()["minted-view"]?.claimToken).toBe(mintedClaim) + expect( + postMessageSpy.mock.calls.map(([message]) => message).filter((m) => m.type === "viewStateClaim"), + ).toHaveLength(1) + }) + + test("re-binds a claim-less legacy entry instead of loading its selections", async () => { + // An entry written before claims existed cannot be attributed to any webview: its + // selections are refused, and the key is re-bound with a fresh host-minted claim so + // the upgrade does not brick per-view persistence. + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { mode: "architect", currentApiConfigName: "profile-a", updatedAt: 1 }, + }) + + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + + await provider["setViewStateId"]("legacy-view") + + // No stored selection is adopted... + expect(provider["viewStateId"]).toBe("legacy-view") + expect(provider["viewLocalState"]).toStrictEqual({}) + + // ...the old entry is replaced by a claim-bearing one with the unbound selections gone... + const rebound = storedEntries()["legacy-view"] + expect(rebound).toEqual({ claimToken: expect.any(String), updatedAt: expect.any(Number) }) + expect(rebound).not.toHaveProperty("mode") + expect(rebound).not.toHaveProperty("currentApiConfigName") + + // ...and the fresh claim is handed to this webview so the next launch can prove ownership. + expect(postMessageSpy).toHaveBeenCalledWith({ + type: "viewStateClaim", + viewStateId: "legacy-view", + viewStateClaim: rebound?.claimToken, + }) + + await provider.dispose() + + const reopened = createProvider() + await reopened["setViewStateId"]("legacy-view", rebound?.claimToken) + + expect(reopened["viewStateId"]).toBe("legacy-view") + expect(reopened["viewLocalState"]).toStrictEqual({}) + + await reopened.dispose() + }) + + test("still refuses an id a live view holds, even when the claim matches", async () => { + const owner = createProvider() + await owner["setViewStateId"]("shared-live-view") + await owner.saveViewState("mode", "architect") + const claim = storedEntries()["shared-live-view"]?.claimToken + + const sibling = createProvider() + + try { + await sibling["setViewStateId"]("shared-live-view", claim) + + // The live-instance guard runs first and keeps working: a second live view never + // shares a key with the view that holds it. + expect(sibling["viewStateId"]).toBe(sibling.viewId) + expect(sibling["viewLocalState"]).toStrictEqual({}) + expect(owner["viewStateId"]).toBe("shared-live-view") + } finally { + await sibling.dispose() + await owner.dispose() + } + }) + }) + + describe("view state persistence edge cases", () => { + it("should read viewStates from the ContextProxy cache when not fresh", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { "stable-sidebar-view": { mode: "architect" } }) + expect(provider["getPersistedViewStates"]()).toEqual({ "stable-sidebar-view": { mode: "architect" } }) + await provider.dispose() + }) + + it("should treat a corrupted non-object viewStates value as an empty map", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // 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 keep the persisted profile name and log when the profile lookup fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + vi.spyOn(provider.providerSettingsManager, "getProfile").mockRejectedValue(new Error("profile missing")) + await provider.saveViewState("currentApiConfigName", "my-profile") + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewLocalState"].currentApiConfigName).toBe("my-profile") + expect(provider["viewLocalState"]).not.toHaveProperty("apiConfiguration") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Unable to resolve API profile 'my-profile'")) + await provider.dispose() + }) + + it("should discard a stale load when the viewStateId changes during the profile lookup", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const logSpy = vi.spyOn(provider, "log") + vi.spyOn(provider.providerSettingsManager, "getProfile").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") + vi.spyOn(provider.customModesManager, "getCustomModes").mockResolvedValue([]) + // 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) + vi.spyOn(provider.customModesManager, "getCustomModes").mockResolvedValue([]) + 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() + }) + }) + + 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.spyOn(provider.providerSettingsManager, "getProfile").mockResolvedValue({ + name: "my-profile", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "model-x", + }) + // 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") + vi.spyOn(provider.customModesManager, "getCustomModes").mockResolvedValue([]) + // 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) + // The real resetCustomModes writes the modes file, so the method is stubbed rather than the manager replaced. + vi.spyOn(provider.customModesManager, "resetCustomModes").mockResolvedValue(undefined) + // 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 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.setValue("currentApiConfigName", "shared-profile") + // Only these three profile-store calls are exercised by the save path, so each is stubbed on the real manager + // instead of replacing it, which keeps the stubs checked against the declared signatures. + vi.spyOn(saver.providerSettingsManager, "saveConfig").mockResolvedValue("shared-id") + vi.spyOn(saver.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "shared-profile", id: "shared-id" }, + ]) + vi.spyOn(saver.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + const providerSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" } + + await saver.upsertProviderProfile("shared-profile", providerSettings, false) + + // The pinned view buffered a copy of the profile's settings; a non-activating save must + // refresh it, or its getState() keeps serving the pre-save configuration. + expect(pinned["viewLocalState"].apiConfiguration).toEqual(providerSettings) + await saver.dispose() + await pinned.dispose() + }) + + it("should refresh the buffered overlay of another view pinned to the activated profile", async () => { + const saver = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const pinned = new ClineProvider( + mockContext, + mockOutputChannel, + "editor", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await pinned.setValue("currentApiConfigName", "shared-profile") + // Only these three profile-store calls are exercised by the save path, so each is stubbed on the real manager + // instead of replacing it, which keeps the stubs checked against the declared signatures. + vi.spyOn(saver.providerSettingsManager, "saveConfig").mockResolvedValue("shared-id") + vi.spyOn(saver.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "shared-profile", id: "shared-id" }, + ]) + vi.spyOn(saver.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + const providerSettings = { apiProvider: providerIdentifiers.anthropic, apiKey: "fresh-key" } + + await saver.upsertProviderProfile("shared-profile", providerSettings, true) + + // The pinned view buffered a copy of the profile's settings; an activating save refreshes + // it through the sibling loop, so this row covers the activate=true branch that the case + // above no longer reaches. + expect(pinned["viewLocalState"].apiConfiguration).toEqual(providerSettings) + await saver.dispose() + await pinned.dispose() + }) + + 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 leave a shared value alone when another view wrote the same value meanwhile", 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(async (key: string) => { + if (key === "viewStates") { + // Another view selects the same mode while this provider's per-view save is in + // flight: the value matches this attempt, but the write does not belong to it. + await provider.contextProxy.setValue("mode", "code") + throw new Error("storage unavailable") + } + return Promise.resolve() + }) + + await expect(provider.setValue("mode", "code")).rejects.toThrow("storage unavailable") + + // The newer write stands. Restoring the captured "architect" would undo the other + // view's successful selection in every view that reads shared state. + expect(provider.getValue("mode")).toBe("code") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + it("should still roll back its own shared write when no newer write exists", 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 ownership token must not become a reason to skip a rollback this provider owns. + expect(provider.getValue("mode")).toBe("architect") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + it("should roll back only the batch keys it still owns after a failed per-view save", 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(async (key: string) => { + if (key === "viewStates") { + await provider.contextProxy.setValue("mode", "code") + throw new Error("storage unavailable") + } + return Promise.resolve() + }) + + await expect(provider.setValues({ mode: "code", currentApiConfigName: "other-config" })).rejects.toThrow( + "storage unavailable", + ) + + // Key-level ownership, not batch-level: the mode a newer view selected survives, while the + // profile name this batch still owns is restored. + expect(provider.getValue("mode")).toBe("code") + expect(provider.getValue("currentApiConfigName")).toBe("current-config") + + mockContext.globalState.update = originalUpdate + await provider.dispose() + }) + it("restores both definitions of a slug that exists as a project and a global mode", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const projectCopy = { + slug: "shared-slug", + name: "Project copy", + roleDefinition: "r", + groups: ["read" as const], + source: "project" as const, + } + const globalCopy = { + slug: "shared-slug", + name: "Global copy", + roleDefinition: "r", + groups: ["read" as const], + source: "global" as const, + } + const manager = provider["customModesManager"] + Object.assign(manager, { + // The merged read hides the global copy behind the project one; the rollback snapshot must + // not, or restoring it would write the project definition to the global file as well. + getCustomModes: vi.fn().mockResolvedValue([projectCopy]), + getCustomModeDefinitions: vi.fn().mockResolvedValue([projectCopy, globalCopy]), + updateCustomMode: vi.fn().mockResolvedValue(undefined), + removeCustomModeEntry: vi.fn().mockResolvedValue(true), + }) + vi.spyOn(provider, "setValues").mockRejectedValue(new Error("configuration write failed")) + + await expect( + provider.createTask( + "text", + undefined, + undefined, + {}, + { customModes: [{ ...globalCopy, name: "Batch copy" }] }, + ), + ).rejects.toThrow("configuration write failed") + + // Both copies are restored, each to its own file, so the batch's version cannot outlive the + // rollback in the global file while the project file gets the pre-batch definition back. + expect(manager.updateCustomMode).toHaveBeenCalledWith("shared-slug", projectCopy) + expect(manager.updateCustomMode).toHaveBeenCalledWith("shared-slug", globalCopy) + + await provider.dispose() + }) + it("should remove a custom mode the batch registered when the batch fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const savedMode = { slug: "existing-mode", name: "Existing", roleDefinition: "r", groups: ["read" as const] } + // The batch hands in a different definition for the slug that already exists, so the restore + // call below cannot be satisfied by the registration call that preceded it. + const existingMode = { ...savedMode, name: "Overridden", roleDefinition: "changed" } + const newMode = { slug: "new-mode", name: "New", roleDefinition: "r", groups: ["read" as const] } + const manager = provider["customModesManager"] + Object.assign(manager, { + getCustomModes: vi.fn().mockResolvedValue([savedMode]), + // The rollback snapshot reads both source files, so the double answers that reader too. + getCustomModeDefinitions: vi.fn().mockResolvedValue([savedMode]), + updateCustomMode: vi.fn().mockResolvedValue(undefined), + removeCustomModeEntry: vi.fn().mockResolvedValue(true), + }) + vi.spyOn(provider, "setValues").mockRejectedValue(new Error("configuration write failed")) + + await expect( + provider.createTask("text", undefined, undefined, {}, { customModes: [existingMode, newMode] }), + ).rejects.toThrow("configuration write failed") + + // The slug that already existed is restored to its earlier definition; the slug this batch + // introduced is removed, and only through the rules-folder-safe path. + expect(manager.updateCustomMode).toHaveBeenCalledWith("existing-mode", savedMode) + expect(manager.removeCustomModeEntry).toHaveBeenCalledWith("new-mode") + expect(manager.removeCustomModeEntry).not.toHaveBeenCalledWith("existing-mode") + + await provider.dispose() + }) + it("should roll back the modes a registration left half-installed", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const savedMode = { slug: "existing-mode", name: "Existing", roleDefinition: "r", groups: ["read" as const] } + // The batch re-registers the slug that already exists under a different definition, so a + // restore call is distinguishable from the registration call. + const overriddenMode = { ...savedMode, name: "Overridden", roleDefinition: "changed" } + const newMode = { slug: "new-mode", name: "New", roleDefinition: "r", groups: ["read" as const] } + const calls: Array<[string, string]> = [] + const manager = provider["customModesManager"] + Object.assign(manager, { + getCustomModes: vi.fn().mockResolvedValue([savedMode]), + // The rollback snapshot reads both source files, so the double answers that reader too. + getCustomModeDefinitions: vi.fn().mockResolvedValue([savedMode]), + updateCustomMode: vi.fn((slug: string, mode: typeof savedMode) => { + calls.push([slug, mode.name]) + // The batch's second registration is the one that throws. + if (slug === "new-mode") { + return Promise.reject(new Error("mode registration failed")) + } + return Promise.resolve(undefined) + }), + removeCustomModeEntry: vi.fn().mockResolvedValue(true), + }) + const setValuesSpy = vi.spyOn(provider, "setValues").mockResolvedValue(undefined) + + await expect( + provider.createTask("text", undefined, undefined, {}, { customModes: [overriddenMode, newMode] }), + ).rejects.toThrow("mode registration failed") + + // Registration order first, then the rollback: the slug that already existed is rewritten to + // the snapshot taken before the batch, and the slug this batch introduced is removed through + // the rules-folder-safe path. Nothing downstream of the failed registration runs. + expect(calls).toEqual([ + ["existing-mode", "Overridden"], + ["new-mode", "New"], + ["existing-mode", "Existing"], + ]) + expect(manager.removeCustomModeEntry).toHaveBeenCalledWith("new-mode") + expect(manager.removeCustomModeEntry).not.toHaveBeenCalledWith("existing-mode") + expect(setValuesSpy).not.toHaveBeenCalled() + + await provider.dispose() + }) + + it("should keep a prototype key carried by a payload out of the rollback write", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("mode", "code") + // A payload-shaped value can carry an own "__proto__" next to the settings it names, and both + // maps handed to the rollback are built from those same keys. + const attempted = Object.fromEntries([ + ["__proto__", { polluted: true }], + ["mode", "code"], + ]) as RooCodeSettings + const previous = Object.fromEntries([ + ["__proto__", { polluted: true }], + ["mode", "architect"], + ]) as RooCodeSettings + const rollbackWrite = vi.spyOn(provider["contextProxy"], "setValues") + + await provider["_restoreSharedValuesAfterFailedViewWrite"](previous, attempted) + + expect(rollbackWrite).toHaveBeenCalledTimes(1) + const written = rollbackWrite.mock.calls[0][0] as Record + expect(Object.keys(written)).toEqual(["mode"]) + expect(Object.getPrototypeOf(written)).toBe(Object.prototype) + expect(written["polluted"]).toBeUndefined() + expect(({} as Record).polluted).toBeUndefined() + expect(provider.getValue("mode")).toBe("architect") + + await provider.dispose() + }) }) describe("webviewMessageHandler no-floating-promises coverage", () => {