diff --git a/packages/types/src/__tests__/global-settings.test.ts b/packages/types/src/__tests__/global-settings.test.ts index f2b2e7e8e5..e9efe69c42 100644 --- a/packages/types/src/__tests__/global-settings.test.ts +++ b/packages/types/src/__tests__/global-settings.test.ts @@ -3,6 +3,7 @@ import { DEFAULT_DESTRUCTIVE_COMMAND_GUARD_ENABLED, GLOBAL_SETTINGS_KEYS, globalSettingsSchema, + viewStateSchema, } from "../global-settings.js" describe("destructive command guard global setting", () => { @@ -38,7 +39,6 @@ describe("alwaysDenyUnapprovedCommands global setting", () => { expect(() => globalSettingsSchema.parse({ alwaysDenyUnapprovedCommands: "true" })).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 }) @@ -51,3 +51,85 @@ describe("collapseCommandOutputByDefault global setting", () => { expect(globalSettingsSchema.safeParse({ collapseCommandOutputByDefault: "true" }).success).toBe(false) }) }) + +describe("viewStates global setting", () => { + it("preserves each webview instance selection through the persisted schema", () => { + const parsed = globalSettingsSchema.parse({ + viewStates: { + "session-1": { mode: "code", currentApiConfigName: "default", updatedAt: 1700000000000 }, + "session-2": { mode: "architect" }, + }, + }) + // Every field has to survive the round trip: a selection the schema drops is + // written back as absent, so the next window opens in the default mode. + expect(parsed.viewStates).toEqual({ + "session-1": { mode: "code", currentApiConfigName: "default", updatedAt: 1700000000000 }, + "session-2": { mode: "architect" }, + }) + + // The key list is part of the contract: a setting that never reaches + // GLOBAL_SETTINGS_KEYS is not a persisted setting, whatever the schema does. + expect(GLOBAL_SETTINGS_KEYS).toContain("viewStates") + }) + + it("accepts an empty entry because every field is optional", () => { + expect(() => globalSettingsSchema.parse({ viewStates: { "view-1": {} } })).not.toThrow() + // The control that makes the line above a test: an absent viewStates is omitted + // rather than defaulted, so an empty entry is a record and not the same shape + // as nothing at all. + expect(globalSettingsSchema.parse({}).viewStates).toBeUndefined() + }) + + it("parses a single view state on its own", () => { + expect(viewStateSchema.parse({ mode: "ask", currentApiConfigName: "x", updatedAt: 5 })).toEqual({ + mode: "ask", + currentApiConfigName: "x", + updatedAt: 5, + }) + }) + + it("rejects a numeric mode and names the offending path", () => { + // The control first: the same record without the bad field must parse, so a + // failure here can only mean the schema rejected that value - not that the record + // shape is unsupported or the assertion never ran. + expect(globalSettingsSchema.safeParse({ viewStates: { a: { mode: "code" } } }).success).toBe(true) + + const result = globalSettingsSchema.safeParse({ viewStates: { a: { mode: 42 } } }) + + expect(result.success).toBe(false) + expect(result.error?.issues[0]?.path).toEqual(["viewStates", "a", "mode"]) + }) + + it("rejects a non-string currentApiConfigName and names the offending path", () => { + expect(globalSettingsSchema.safeParse({ viewStates: { a: { currentApiConfigName: "x" } } }).success).toBe(true) + + const result = globalSettingsSchema.safeParse({ viewStates: { a: { currentApiConfigName: 7 } } }) + + expect(result.success).toBe(false) + expect(result.error?.issues[0]?.path).toEqual(["viewStates", "a", "currentApiConfigName"]) + }) + + it("rejects a non-numeric updatedAt and names the offending path", () => { + expect(globalSettingsSchema.safeParse({ viewStates: { a: { updatedAt: 1 } } }).success).toBe(true) + + const result = globalSettingsSchema.safeParse({ viewStates: { a: { updatedAt: "1" } } }) + + expect(result.success).toBe(false) + expect(result.error?.issues[0]?.path).toEqual(["viewStates", "a", "updatedAt"]) + }) + + it("rejects a view state that is not an object", () => { + // A bare string under a session id is what a half-migrated store looks like; + // accepting it defers the crash to whoever reads the selection back. + expect(globalSettingsSchema.safeParse({ viewStates: { a: { mode: "code" } } }).success).toBe(true) + + const result = globalSettingsSchema.safeParse({ viewStates: { a: "code" } }) + + expect(result.success).toBe(false) + expect(result.error?.issues[0]?.path).toEqual(["viewStates", "a"]) + + // A null entry fails for a different reason than a string entry: the record + // type tolerates an absent value and rejects a present non-object one. + expect(() => globalSettingsSchema.parse({ viewStates: { "view-1": null } })).toThrow() + }) +}) diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index f5757a51c7..fe896f3ef9 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -114,6 +114,15 @@ 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(), +}) + /** * GlobalSettings */ @@ -122,6 +131,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..9c9791ee72 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -712,6 +712,7 @@ export interface WebviewMessage { | "themeFixtureProbeResponse" | "diagnosticsResponse" text?: string + viewStateId?: string taskId?: string editedMessageContent?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud" diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index e53f60fb83..26a253057e 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -67,6 +67,9 @@ 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 { diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 01695d7994..a7b740e65a 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -55,6 +55,19 @@ export const providerProfilesSchema = z.object({ export type ProviderProfiles = z.infer +/** + * Signals that a profile's configuration no longer exists. Callers that treat an + * already-deleted profile as an idempotent no-op branch on this type instead of + * matching error message text, which a profile name containing the phrase could + * otherwise spoof. + */ +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() @@ -456,7 +469,7 @@ export class ProviderSettingsManager { name = params.name if (!providerProfiles.apiConfigs[name]) { - throw new Error(`Config with name '${name}' not found`) + throw new ProviderSettingsNotFoundError(`Config with name '${name}' not found`) } providerSettings = providerProfiles.apiConfigs[name] @@ -468,7 +481,7 @@ export class ProviderSettingsManager { ) if (!entry) { - throw new Error(`Config with ID '${id}' not found`) + throw new ProviderSettingsNotFoundError(`Config with ID '${id}' not found`) } name = entry[0] @@ -478,10 +491,31 @@ export class ProviderSettingsManager { return { name, ...providerSettings } }) } catch (error) { + // Keep the typed not-found signal: callers branch on it to treat an already + // deleted profile as an idempotent no-op instead of matching message text. + if (error instanceof ProviderSettingsNotFoundError) { + throw error + } throw new Error(`Failed to get profile: ${error instanceof Error ? error.message : error}`) } } + /** + * Read the manager-owned current profile selection. Exposed for callers that + * snapshot the full activation state before a mutation group so a failure can + * compensate the manager store as well as its ContextProxy copies. + */ + public async getCurrentProfileName(): Promise { + try { + return await this.lock(async () => { + const providerProfiles = await this.load() + return providerProfiles.currentApiConfigName + }) + } catch (error) { + throw new Error(`Failed to read the active profile: ${error instanceof Error ? error.message : error}`) + } + } + /** * Activate a profile by name or ID. */ @@ -511,7 +545,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 +556,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..0e4a14611a 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -1170,4 +1170,31 @@ Output only the summary of the conversation so far, without any additional comme expect(customSupportPromptsUpdateCalls.length).toBe(0) }) }) + + describe("export", () => { + it("omits viewStates from the exported global settings", async () => { + // The sibling setting is the control: if the export returned nothing at all - + // a schema failure, an empty value set - the first assertion below fails and the + // absent viewStates cannot be read as a passing test for the wrong reason. + mockGlobalState.get.mockImplementation((key: string) => { + if (key === "viewStates") { + return { "view-1": { mode: "code", currentApiConfigName: "profile-a", updatedAt: 1700000000000 } } + } + if (key === "mode") { + return "code" + } + return undefined + }) + + const exportProxy = new ContextProxy(mockContext) + await exportProxy.initialize() + + const exported = await exportProxy.export() + + expect(exported?.mode).toBe("code") + expect(Object.keys(exported ?? {})).toContain("mode") + expect(exported?.viewStates).toBeUndefined() + expect(Object.keys(exported ?? {})).not.toContain("viewStates") + }) + }) }) diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index a1fc9ac7de..614ff0ed53 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 @@ -1829,4 +1829,111 @@ describe("ProviderSettingsManager", () => { expect(result.activeProfileId).toBe("local-id") }) }) + + describe("profile reads and deletes answer with the type callers branch on", () => { + const profilesWith = (apiConfigs: Record, currentApiConfigName = "default") => + JSON.stringify({ + currentApiConfigName, + apiConfigs, + modeApiConfigs: {}, + migrations: { rateLimitSecondsMigrated: false }, + }) + + it("rejects a name lookup that misses with ProviderSettingsNotFoundError", async () => { + mockSecrets.get.mockResolvedValue(profilesWith({ default: { id: "default" } })) + + const error = await providerSettingsManager.getProfile({ name: "absent" }).catch((cause: unknown) => cause) + + if (!(error instanceof ProviderSettingsNotFoundError)) { + throw new Error(`expected ProviderSettingsNotFoundError, got ${String(error)}`) + } + expect(error.name).toBe("ProviderSettingsNotFoundError") + expect(error.message).toBe("Config with name 'absent' not found") + }) + + it("rejects an id lookup that misses with ProviderSettingsNotFoundError", async () => { + mockSecrets.get.mockResolvedValue(profilesWith({ default: { id: "default" } })) + + const error = await providerSettingsManager + .getProfile({ id: "missing-id" }) + .catch((cause: unknown) => cause) + + if (!(error instanceof ProviderSettingsNotFoundError)) { + throw new Error(`expected ProviderSettingsNotFoundError, got ${String(error)}`) + } + expect(error.message).toBe("Config with ID 'missing-id' not found") + }) + + it("keeps a missed lookup out of the wrapped-read branch", async () => { + mockSecrets.get.mockResolvedValue(profilesWith({ default: { id: "default" } })) + + await expect(providerSettingsManager.getProfile({ name: "absent" })).rejects.not.toThrow( + "Failed to get profile:", + ) + }) + + it("surfaces a failed profile read as a plain Error naming the read", async () => { + mockSecrets.get.mockRejectedValue(new Error("storage unavailable")) + + const error = await providerSettingsManager.getProfile({ name: "default" }).catch((cause: unknown) => cause) + + if (!(error instanceof Error)) { + throw new Error(`expected an Error, got ${String(error)}`) + } + expect(error).not.toBeInstanceOf(ProviderSettingsNotFoundError) + expect(error.name).toBe("Error") + expect(error.message.startsWith("Failed to get profile:")).toBe(true) + expect(error.message.includes("storage unavailable")).toBe(true) + }) + + it("rethrows a missing config from deleteConfig as the typed error", async () => { + mockSecrets.get.mockResolvedValue(profilesWith({ default: { id: "default" }, other: { id: "other-id" } })) + + const error = await providerSettingsManager.deleteConfig("absent").catch((cause: unknown) => cause) + + if (!(error instanceof ProviderSettingsNotFoundError)) { + throw new Error(`expected ProviderSettingsNotFoundError, got ${String(error)}`) + } + expect(error.message).toBe("Config 'absent' not found") + }) + + it("keeps the last-remaining-config refusal a plain Error under the delete prefix", async () => { + mockSecrets.get.mockResolvedValue(profilesWith({ default: { id: "default" } })) + + const error = await providerSettingsManager.deleteConfig("default").catch((cause: unknown) => cause) + + if (!(error instanceof Error)) { + throw new Error(`expected an Error, got ${String(error)}`) + } + expect(error).not.toBeInstanceOf(ProviderSettingsNotFoundError) + expect(error.message).toBe("Failed to delete config: Error: Cannot delete the last remaining configuration") + }) + }) + + describe("getCurrentProfileName", () => { + it("returns the profile the manager store marks current", async () => { + mockSecrets.get.mockResolvedValue( + JSON.stringify({ + currentApiConfigName: "work", + apiConfigs: { default: { id: "default" }, work: { id: "work-id" } }, + modeApiConfigs: {}, + migrations: { rateLimitSecondsMigrated: false }, + }), + ) + + await expect(providerSettingsManager.getCurrentProfileName()).resolves.toBe("work") + }) + + it("wraps a failed read in an Error naming the active profile", async () => { + mockSecrets.get.mockRejectedValue(new Error("storage unavailable")) + + const error = await providerSettingsManager.getCurrentProfileName().catch((cause: unknown) => cause) + + if (!(error instanceof Error)) { + throw new Error(`expected an Error, got ${String(error)}`) + } + expect(error).not.toBeInstanceOf(ProviderSettingsNotFoundError) + expect(error.message.startsWith("Failed to read the active profile:")).toBe(true) + }) + }) }) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 6c66655325..60607f0d3b 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -2806,4 +2806,60 @@ describe("importExport", () => { }, ) }) + + describe("viewStates import skip", () => { + const importedWith = async (globalSettings: Record) => { + ;(fs.readFile as Mock).mockResolvedValue( + JSON.stringify({ + providerProfiles: { + currentApiConfigName: "test", + apiConfigs: { test: { apiProvider: providerIdentifiers.openai, id: "test-id" } }, + }, + globalSettings, + }), + ) + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "test", id: "test-id", apiProvider: providerIdentifiers.openai }, + ]) + + const result = await importSettingsFromPath("/mock/path/settings.json", { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + return { + warnings: (result as { warnings?: string[] }).warnings, + written: mockContextProxy.setValues.mock.calls[0]?.[0], + } + } + + it("skips viewStates without a warning and keeps the sibling setting", async () => { + const { warnings, written } = await importedWith({ + mode: "code", + viewStates: { "view-1": { mode: "code", updatedAt: 1700000000000 } }, + }) + + // The sibling is the control: if the skip swallowed every key the first line + // below fails, so an absent viewStates on its own cannot pass for the wrong + // reason. + expect(Object.keys(written ?? {})).toContain("mode") + expect(Object.keys(written ?? {})).not.toContain("viewStates") + // The skip is deliberate, so it must not arrive dressed as an "Unknown setting" + // warning: the key is known and deliberately not exported, not junk. + expect(warnings?.some((warning) => warning.includes("viewStates"))).toBeFalsy() + }) + + it("keeps the sibling setting when viewStates is absent", async () => { + const { warnings, written } = await importedWith({ mode: "code" }) + + expect(Object.keys(written ?? {})).toContain("mode") + expect(Object.keys(written ?? {})).not.toContain("viewStates") + expect(warnings?.some((warning) => warning.includes("viewStates"))).toBeFalsy() + }) + }) }) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 9882039ad3..29c9497552 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -98,6 +98,14 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): { for (const [key, rawValue] of Object.entries(rawGlobalSettings)) { const path = `globalSettings.${key}` + + // Per-view selection state is machine-local: it round-trips through the + // normal runtime and pruning paths, but importing it would pin selections + // from another machine's views on this one. + if (key === "viewStates") { + continue + } + const schema = globalSettingsShape[key as keyof GlobalSettings] if (!schema) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6cec3bdbe7..7e24d5240b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -160,6 +160,21 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" +type PersistedViewState = NonNullable[string] + +/** + * Values that can be held in a view-local state buffer (in-memory) and, for the + * non-secret subset, persisted durably per stable view id. + */ +type ViewLocalStateValues = Partial & Partial + +/** + * The keys the view-local mutation helper actually applies. A wider key would update + * neither the in-memory buffer nor the persisted entry, so it must not be accepted by + * the public save path. + */ +type PerViewSavedStateKey = "mode" | "currentApiConfigName" | "apiConfiguration" + /** * 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 +263,22 @@ 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() + + /** + * View ids whose durable entry a queued write intentionally removed + * (unpin, clear, or re-key move). Activation rollback treats these as + * tombstones and never resurrects a tracked eviction the clear has + * since deleted; a write that lands an entry lifts its tombstone. + */ + private static clearedViewStateIds = new Set() + + // Incremented when a reset clears the durable map; a queued write that started before the + // reset must not persist its entry after the clear. + private static viewStatesResetGeneration = 0 private webviewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< string, @@ -415,7 +446,64 @@ 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 identifier for persisted per-view state keys. + * Defaults to viewId until the webview reports its VS Code-persisted id. + */ + 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 = {} + + /** + * View ids evicted from the durable viewStates map by this instance's writes + * since the last activation snapshot: the rollback restores exactly these + * entries, so a sibling's intentional pin clear is never resurrected. + */ + private prunedViewIdsDuringActivation: string[] = [] + + /** + * 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 + + // Callers that must actually deliver a webview message wait on this gate. The + // SidebarProvider.focus command settles when VS Code opens the pane - well before + // resolveWebviewView hands this provider its view - and postMessageToWebview returns + // without sending while `view` is unset, so a message posted straight after that command + // can be dropped. The gate settles when the view resolves and on every path that removes + // it, so a caller can never hang on a view that is never coming. + private viewResolvedGate?: Promise + private viewResolvedGateRelease?: () => void public settingsImportedAt?: number public readonly latestAnnouncementId = "nov-2026-v3.96.0-settings-costs-reliability" // v3.96.0 chat settings, provider costs, and reliability public readonly providerSettingsManager: ProviderSettingsManager @@ -430,6 +518,11 @@ export class ClineProvider mdmService?: MdmService, ) { super() + // Initialize viewId based on renderContext and monotonically increasing instance identifier for uniqueness. + // activeInstances is used for visibility/iteration checks, so we keep tracking instances separately. + this.viewId = `${renderContext}-${ClineProvider.nextViewId++}` + this.viewStateId = this.viewId + ClineProvider.activeInstances.add(this) this.currentWorkspacePath = getWorkspacePath() this.pendingEditOperations = new PendingEditOperationStore( ClineProvider.PENDING_OPERATION_TIMEOUT_MS, @@ -463,7 +556,17 @@ export class ClineProvider await this.postStateToWebviewWithoutClineMessages() }) - // Initialize MCP Hub through the singleton manager + // Load initial state from global state into viewLocalState buffer after dependencies used by getState are ready. + void this.loadViewState() + + // Initialize MCP Hub through the singleton manager. The hub can resolve after this + // provider is already disposed (tab-creation rollback disposes immediately), and by + // then disposal has removed the provider from McpServerManager.providers, so a late + // hub must not be attached or registered: nothing would ever unregister it. + // registerClient only happens in this continuation, so skipping it leaves nothing to + // compensate. Disposal deliberately does not await this promise - getInstance waits + // on hub.waitUntilReady(), so a hung readiness wait would block teardown; making the + // late resolution inert is what finalizes it safely. McpServerManager.getInstance(this.context, this) .then((hub) => { this.mcpHub = hub @@ -599,6 +702,535 @@ export class ClineProvider } } + /** + * Override EventEmitter's on method to match TaskProviderLike interface + */ + 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) + } + + /** + * Persists this view's non-secret selections through the serialized write queue. + * The write re-reads the map fresh and merges into the existing entry, removing the + * entry entirely when nothing persistable remains, so concurrent views cannot clobber it. + * The entry is keyed by the view id active when the change was made. Writes captured + * while the provider still holds its temporary (pre-launch) id persist under that id + * and are re-keyed to the stable view id when the webview registers one, so a change + * that lands before the launch message stays durable instead of being lost. + */ + private async savePersistedViewState(values: Partial): Promise { + const resetGeneration = ClineProvider.viewStatesResetGeneration + // Capture the id at change time: a write belongs to the view that was active + // when the change was made, even if a newer id is registered while it is queued. + const viewStateId = this.viewStateId + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + // A reset that ran while this write was queued cleared the durable map: persisting + // the pre-reset pin now would resurrect it on the next reload. + if (ClineProvider.viewStatesResetGeneration !== resetGeneration) { + return + } + + const states = this.getPersistedViewStates({ fresh: true }) + // The first write to an unowned temporary id claims the slot instead of merging + // into it: temporary names restart from 0 on every host start, so an entry under + // this name can be a leftover from an earlier session, and merging would adopt its + // fields and later re-key them under the stable id. Entries under a stable id, or + // under the temporary id this instance already authored, keep the merge semantics. + const ownsTargetEntry = viewStateId !== this.viewId || this.wroteUnderTemporaryViewStateId + const current = ownsTargetEntry ? (states[viewStateId] ?? {}) : {} + const next: PersistedViewState = { ...current } + + if ("mode" in values) { + if (values.mode === undefined || values.mode === null) { + delete next.mode + } else { + next.mode = values.mode + } + } + + if ("currentApiConfigName" in values) { + if (values.currentApiConfigName === undefined || values.currentApiConfigName === null) { + delete next.currentApiConfigName + } else { + next.currentApiConfigName = values.currentApiConfigName + } + } + + let wroteEntry = false + // Tombstone and eviction record durable outcomes, so they are captured here and applied + // only after the write below lands - the rule the authorship flag already follows. + let tombstoned = false + if (!next.mode && !next.currentApiConfigName) { + delete states[viewStateId] + tombstoned = true + } else { + next.updatedAt = values.updatedAt ?? Date.now() + states[viewStateId] = next + wroteEntry = true + } + + const pruned = this.prunePersistedViewStates(states) + const evicted = Object.keys(states).filter((viewId) => !Object.hasOwn(pruned, viewId)) + + try { + await this.contextProxy.setValue("viewStates", pruned) + } catch (error) { + // updateGlobalState applies the new map to the in-memory cache before + // the durable write, so a rejected write leaves the cache claiming an + // entry that never reached disk. Restore the cache to the value durable + // storage actually holds (undefined when the key was never written); the + // cache update lands even if this compensating write also fails, so + // readers cannot observe the lost entry either way. + try { + await this.contextProxy.setValue("viewStates", this.context.globalState.get("viewStates")) + } catch { + // The cache is updated before the durable attempt: it matches the + // durable value regardless of whether this restore write landed. + } + throw error + } + if (tombstoned) { + ClineProvider.clearedViewStateIds.add(viewStateId) + } else { + ClineProvider.clearedViewStateIds.delete(viewStateId) + } + if (evicted.length > 0) { + this.prunedViewIdsDuringActivation.push(...evicted) + } + // 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 + } + + /** + * 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] + try { + await this.contextProxy.setValue("viewStates", states) + // The tombstone is recorded only after the durable delete lands: a rejected write + // leaves the entry in storage, and tombstoning an id that is still persisted would + // let a later activation rollback skip a pin that pruning had evicted. + ClineProvider.clearedViewStateIds.add(viewStateId) + } catch (error) { + // updateGlobalState applies the new map to the in-memory cache before + // the durable write, so a rejected write leaves the cache missing an + // entry that durable storage still holds. Restore the cache to the + // value storage actually holds (undefined when the key was never + // written); the cache update lands even if this compensating write + // also fails, so readers cannot observe the loss either way. + try { + await this.contextProxy.setValue("viewStates", this.context.globalState.get("viewStates")) + } catch { + // The cache is updated before the durable attempt: it matches the + // durable value regardless of whether this restore write landed. + } + throw error + } + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** + * Re-points persisted view pins that reference a removed profile so views do not + * rehydrate a missing profile name after a reload. Runs through the serialized + * write queue like every other viewStates mutation. + */ + private async repointPersistedViewStates( + removedProfileName: string, + replacementProfileName: string, + ): Promise { + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + let changed = false + + for (const [viewId, entry] of Object.entries(states)) { + if (entry?.currentApiConfigName !== removedProfileName) { + continue + } + + changed = true + const { currentApiConfigName: _removed, ...rest } = entry + + if (rest.mode) { + states[viewId] = { ...rest, currentApiConfigName: replacementProfileName, updatedAt: Date.now() } + } else { + states[viewId] = { currentApiConfigName: replacementProfileName, updatedAt: Date.now() } + } + } + + if (changed) { + const pruned = this.prunePersistedViewStates(states) + const evicted = Object.keys(states).filter((viewId) => !Object.hasOwn(pruned, viewId)) + try { + await this.contextProxy.setValue("viewStates", pruned) + } catch (error) { + // updateGlobalState applies the new map to the in-memory cache + // before the durable write: restore the cache to the value durable + // storage actually holds so closed-view pins cannot serve re-pointed + // names that never reached disk, then let the caller compensate. + try { + await this.contextProxy.setValue("viewStates", this.context.globalState.get("viewStates")) + } catch { + // The cache is updated before the durable attempt: it matches the + // durable value regardless of whether this restore write landed. + } + throw error + } + // Evictions are recorded only for a map that actually reached disk: a rejected + // re-point leaves the old names durable, so recording an eviction here would let a + // rollback restore entries that were never removed. + if (evicted.length > 0) { + this.prunedViewIdsDuringActivation.push(...evicted) + } + } + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** + * Keeps only the most recently updated entries of the persisted view states map, + * bounded by MAX_PERSISTED_VIEW_STATES so the global key cannot grow unboundedly. + */ + private prunePersistedViewStates(states: Record): Record { + // Entries of live views must never age out: a view that keeps its mode and profile does + // not refresh updatedAt. The cap applies to closed-view entries only. + const liveViewStateIds = new Set( + Array.from(ClineProvider.activeInstances, (instance) => instance.viewStateId).filter( + (viewStateId): viewStateId is string => Boolean(viewStateId), + ), + ) + const liveEntries = Object.entries(states).filter(([viewStateId]) => liveViewStateIds.has(viewStateId)) + const closedEntries = Object.entries(states) + .filter(([viewStateId]) => !liveViewStateIds.has(viewStateId)) + .sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)) + .slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES) + + return Object.fromEntries([...liveEntries, ...closedEntries]) + } + + /** + * Re-keys this provider's temporary pre-launch viewStates entry to the newly + * registered stable id so pre-launch writes become durable under the stable key + * instead of orphaning under a session-local temporary id. Only the provider's own + * temporary id is eligible: an entry under a previously registered stable id belongs + * to that webview's storage and is left alone. An entry that is already under the + * temporary id without this provider having written it is an orphan from an earlier + * session and is cleared rather than re-keyed. When the stable entry already exists + * it wins and the temporary entry is dropped, because temporary ids are session + * counters that can collide across window reloads. Runs through the serialized write + * queue like every other viewStates mutation. + */ + 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 }) + // Own-property checks only: the map is a plain object, and an inherited + // Object.prototype member must never look like a persisted entry (a map + // written by an older build can still carry a "constructor"-style key). + const previous = Object.prototype.hasOwnProperty.call(states, previousViewStateId) + ? states[previousViewStateId] + : undefined + + if (!previous) { + return + } + + delete states[previousViewStateId] + + const nextEntryExists = Object.prototype.hasOwnProperty.call(states, nextViewStateId) + if (!nextEntryExists) { + states[nextViewStateId] = previous + } + + const pruned = this.prunePersistedViewStates(states) + const evicted = Object.keys(states).filter((viewId) => !Object.hasOwn(pruned, viewId)) + + try { + await this.contextProxy.setValue("viewStates", pruned) + } catch (error) { + // updateGlobalState applies the new map to the in-memory cache before + // the durable write, so a rejected re-key would leave the cache serving + // the stable entry while durable storage still holds the temporary one. + // Restore the cache to the value durable storage actually holds (the same + // rollback savePersistedViewState applies), so a later load or a retry of + // the registration sees the pre-rekey entry. + try { + await this.contextProxy.setValue("viewStates", this.context.globalState.get("viewStates")) + } catch { + // The cache is updated before the durable attempt: it matches the + // durable value regardless of whether this restore write landed. + } + throw error + } + + ClineProvider.clearedViewStateIds.add(previousViewStateId) + if (!nextEntryExists) { + ClineProvider.clearedViewStateIds.delete(nextViewStateId) + } + if (evicted.length > 0) { + this.prunedViewIdsDuringActivation.push(...evicted) + } + // 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 + } + + /** + * Registers this provider's stable view identifier and loads any persisted selections it owns. + * The identifier is sanitized so it remains a safe object key in the shared viewStates map. + */ + public async setViewStateId(viewStateId: string | undefined): 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 every Object.prototype member name ("__proto__", "constructor", "toString", ...): + // states[normalizedViewStateId] would resolve to an inherited value instead of a + // persisted entry, so the stable entry would never be written while the pre-launch + // entry is deleted - losing the selection. + Object.prototype.hasOwnProperty.call(Object.prototype, normalizedViewStateId) + ) { + 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 + } + + 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() + } 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 + } + } + + /** + * Loads non-secret persisted selections from the registered viewStates map. + * Missing entries are intentionally left unset so getState() falls back to shared ContextProxy values. + * Fields mutated while the async profile lookup is in flight are reapplied on top of the + * loaded state, field by field, so in-flight user selections are not clobbered by the load. + */ + private async loadViewState(): Promise { + // Reset/import clears the buffer without changing the view id; a load that completes + // after such a clear must not republish the state it captured before the reset. + const loadedGeneration = this.viewLocalStateGeneration + // Capture the id this load is for: a newer id registered while an async + // profile lookup is in flight must not be overwritten by this stale load. + const loadedForViewId = this.viewStateId + + // While the stable id has not been registered yet, the key is this instance's + // session-local name. Temporary names restart from 0 on every host start, so an + // entry under the same name can belong to a different session. Load only entries + // this instance authored. + if (loadedForViewId === this.viewId && !this.wroteUnderTemporaryViewStateId) { + return + } + try { + const persisted: PersistedViewState | undefined = this.getPersistedViewStates()[loadedForViewId] + + const loadedState: Partial = {} + + // Snapshot the in-memory buffer before the async profile lookup. The + // mutation paths update viewLocalState in place, so a shallow copy is + // what makes fields mutated during the load window observable below. + const preLoadBuffer = { ...this.viewLocalState } + + if (persisted?.mode) { + // A persisted mode may reference a custom mode that was deleted after it was + // pinned: restore it only when the slug still resolves, so a stale slug cannot + // 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 + } + + // Reapply the buffer fields that changed while the load was in flight, + // tracking the change instead of testing against undefined: a field cleared + // during the load window must stay cleared (the loaded value must not + // resurrect it), and a field written to a new value must win over it. + // Untouched fields keep the persisted values authoritative, and the + // pre-load buffer is never merged wholesale so stale temporary-id state + // cannot override the stable persisted state. + const postLoadBuffer = this.viewLocalState + const mergedState: Partial = { ...loadedState } + + // The lookup resolved the profile that was pinned when the load started. If the view + // pinned a different profile during the lookup window, those settings belong to a + // profile this view no longer names, and keeping them would report the new name with + // the old profile's configuration. Dropping the overlay falls back to the shared + // store, which is the same outcome as any other unresolved view-local profile. + if ( + persisted?.currentApiConfigName && + !Object.is(preLoadBuffer.currentApiConfigName, postLoadBuffer.currentApiConfigName) && + !Object.is(persisted.currentApiConfigName, postLoadBuffer.currentApiConfigName) + ) { + delete mergedState.apiConfiguration + } + + 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 profile lookup was in flight: + // republishing here would restore the pre-reset profile 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)}`, + ) + } + } + + /** + * Saves a single view-local state value. The in-memory buffer is always updated; the + * non-secret subset (mode, currentApiConfigName) is persisted durably under the view + * id active when the change was made, re-keyed to the stable id on registration. + * + * Only the three per-view fields are accepted: the mutation helper applies exactly + * those, so any other key would leave both the buffer and the persisted store + * untouched while the method still reported a successful save. + */ + public async saveViewState( + key: K, + value: ViewLocalStateValues[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 */ @@ -3701,7 +4333,64 @@ export class ClineProvider } public async setValue(key: K, value: RooCodeSettings[K]) { + const previousSharedValues = { [key]: this.contextProxy.getValue(key) } as RooCodeSettings await this.contextProxy.setValue(key, value) + try { + await this._saveViewLocalStateFromMutation({ [key]: value }) + } catch (error) { + // The per-view record did not land: restore the prior shared value so the two + // persistence layers cannot diverge, then surface the original failure. + await this._restoreSharedValuesAfterFailedViewWrite(previousSharedValues, { + [key]: value, + } as RooCodeSettings) + throw error + } + } + + /** + * 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. + * + * A key is restored only while the shared store still holds the value this call + * wrote: another view may have committed a newer value for that key in the window + * between the two writes, and writing the pre-mutation snapshot back would undo it. + */ + private async _restoreSharedValuesAfterFailedViewWrite( + previous: RooCodeSettings, + written: RooCodeSettings, + ): Promise { + const restore = Object.fromEntries( + Object.entries(previous).filter(([key]) => + Object.is( + this.contextProxy.getValue(key as keyof RooCodeSettings), + written[key as keyof RooCodeSettings], + ), + ), + ) as RooCodeSettings + + if (Object.keys(restore).length === 0) { + // Every key was superseded by a newer write: there is nothing left of this call to + // undo. + return + } + + try { + await this.contextProxy.setValues(restore) + + // The shared store is back on the pre-mutation values while the per-view record never landed; + // say so: the caller only sees the original per-view failure, and a silent rollback would make the + // divergence between the two stores untraceable. + this.log( + `[ClineProvider] The per-view write failed; the shared value(s) were restored: ${Object.keys( + restore, + ).join(", ")}`, + ) + } 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) { @@ -3713,11 +4402,172 @@ export class ClineProvider } public async setValues(values: RooCodeSettings) { - await this.contextProxy.setValues( - withoutInvalidDisplaySettings(values, (key) => this.log(`[setValues] Ignored an invalid value for ${key}`)), + // Main's display gate stays in front of ours: an invalid display value is dropped + // before anything reaches either persistence layer, and the drop is logged. Our mode + // gate below is a different key class and validates against the custom-modes registry. + 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 + 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. + await this._restoreSharedValuesAfterFailedViewWrite(previousSharedValues, sanitizedValues) + throw error + } + } + + /** + * Persists the view-local subset of a ContextProxy mutation, then updates the in-memory + * viewLocalState buffer. Persistence is awaited first so a failed durable write cannot + * leave the local cache ahead of the persisted state. + */ + private async _saveViewLocalStateFromMutation( + values: Partial & Partial, + ): Promise { + await this._persistViewLocalStateFromMutation(values) + this._updateViewLocalStateFromMutation(values) } + /** + * 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 + } + } + + // Flat provider-settings keys (PROVIDER_SETTINGS_KEYS) are shared settings: + // they are written through the ContextProxy above and must NOT be merged + // into viewLocalState.apiConfiguration, which would turn them into a + // per-view override masking later shared updates from other views. + } + + /** + * Writes the durably persisted subset of a mutation (mode and currentApiConfigName) + * into the registered viewStates map for this view. + */ + 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 view-local state cache so that getState() falls back to ContextProxy defaults. + */ + private _clearViewLocalState(): void { + this.viewLocalStateGeneration += 1 + this.viewLocalState = {} + } + + /** + * Clears the per-view `mode` override of every live view that pinned `slug`. + * + * Deleting a custom mode only moves the acting view to the default mode. A sibling that + * pinned the deleted slug still holds it in its own overlay and in its own durable + * viewStates entry, so that view's getState() reports a mode that no longer exists and any + * task it creates writes that slug into its history. Only overrides that name the deleted + * slug are cleared - a sibling serving another mode keeps its own - and each affected view + * gets its own postStateToWebview, the same fan-out resetState() uses. The acting view is + * the caller's business: its reset goes through setValue, which updates its own overlay. + */ + async clearViewLocalModeForDeletedSlug(slug: string): Promise { + for (const instance of ClineProvider.getAllInstances()) { + if (instance === this) { + continue + } + + // Direct private access: compile-time safe across sibling instances. + if (instance.viewLocalState.mode !== slug) { + continue + } + + try { + await instance._saveViewLocalStateFromMutation({ mode: undefined }) + } catch (error) { + instance.log( + `[clearViewLocalModeForDeletedSlug] Failed to clear the mode override for '${slug}': ${ + error instanceof Error ? error.message : String(error) + }`, + ) + continue + } + + try { + await instance.postStateToWebview() + } catch (error) { + instance.log( + `[clearViewLocalModeForDeletedSlug] Failed to post state after clearing the mode override for '${slug}': ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + } + } + + /** + * Broadcast a reset/import invalidation to all live ClineProvider instances, clearing + * both in-memory view-local caches and durable per-view selections so stale view state + * cannot mask imported/reset shared state after reload. + */ + async broadcastResetToAllInstances(): Promise {} + // dev async resetState() { diff --git a/src/core/webview/__tests__/ClineProvider.persistedViewState.spec.ts b/src/core/webview/__tests__/ClineProvider.persistedViewState.spec.ts new file mode 100644 index 0000000000..cb5c83da38 --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.persistedViewState.spec.ts @@ -0,0 +1,492 @@ +import { WebviewFocusTracker } from "../WebviewFocusTracker" +// pnpm --dir src exec vitest run --config vitest.core.config.ts core/webview/__tests__/ClineProvider.persistedViewState.spec.ts + +import * as vscode from "vscode" +import { makeCompositeDisposable } from "../../../test-utils/vscode" + +import { TelemetryService } from "@roo-code/telemetry" +import type { GlobalState, ModeConfig } from "@roo-code/types" + +import { ContextProxy } from "../../config/ContextProxy" +import { ClineProvider } from "../ClineProvider" + +// Mock setup +vi.mock("fs/promises", () => ({ + mkdir: vi.fn().mockResolvedValue(undefined), + writeFile: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue(""), + unlink: vi.fn().mockResolvedValue(undefined), + rmdir: vi.fn().mockResolvedValue(undefined), +})) + +vi.mock("../../../utils/storage", () => ({ + getSettingsDirectoryPath: vi.fn().mockResolvedValue("/test/settings/path"), + getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/task/path"), + getGlobalStoragePath: vi.fn().mockResolvedValue("/test/storage/path"), +})) + +vi.mock("p-wait-for", () => ({ + __esModule: true, + default: vi.fn().mockResolvedValue(undefined), +})) + +vi.mock("delay", () => { + const delayFn = (_ms: number) => Promise.resolve() + delayFn.createDelay = delayFn + delayFn.reject = delayFn + delayFn.range = delayFn + return { default: delayFn } +}) + +vi.mock("vscode", () => ({ + ExtensionContext: vi.fn(), + OutputChannel: vi.fn(), + WebviewView: vi.fn(), + Disposable: { from: (...subscriptions: vscode.Disposable[]) => makeCompositeDisposable(...subscriptions) }, + Uri: { + joinPath: vi.fn(), + file: vi.fn(), + }, + commands: { + executeCommand: vi.fn().mockResolvedValue(undefined), + }, + window: { + showInformationMessage: vi.fn(), + showWarningMessage: vi.fn(), + showErrorMessage: vi.fn(), + onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })), + }, + workspace: { + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn().mockReturnValue([]), + update: vi.fn(), + }), + onDidChangeConfiguration: vi.fn().mockImplementation(() => { + return { + dispose: vi.fn(), + } + }), + }, + env: { + uriScheme: "vscode", + language: "en", + appName: "Visual Studio Code", + }, + extensions: { + all: [], + }, + ExtensionMode: { + Production: 1, + Development: 2, + Test: 3, + }, + version: "1.85.0", +})) + +vi.mock("../../../utils/tts", () => ({ + setTtsEnabled: vi.fn(), + setTtsSpeed: vi.fn(), +})) + +vi.mock("../../../api", () => ({ + buildApiHandler: vi.fn(), +})) + +vi.mock("../../../integrations/workspace/WorkspaceTracker", () => { + return { + default: vi.fn().mockImplementation(function () { + return { + initializeFilePaths: vi.fn(), + dispose: vi.fn(), + } + }), + } +}) + +vi.mock("../../task/Task", () => ({ + Task: vi.fn().mockImplementation(() => ({ + taskId: "mock-task-id", + instanceId: "mock-instance-id", + abortTask: vi.fn().mockResolvedValue(undefined), + emit: vi.fn(), + })), +})) + +vi.mock("@roo-code/cloud", () => ({ + CloudService: { + hasInstance: vi.fn().mockReturnValue(true), + get instance() { + return { + isAuthenticated: vi.fn().mockReturnValue(false), + off: vi.fn(), + } + }, + }, + getRooCodeApiUrl: vi.fn().mockReturnValue("https://app.roocode.com"), +})) + +vi.mock("../../../integrations/theme/getTheme", () => ({ + getTheme: vi.fn().mockResolvedValue({ kind: "dark" }), +})) + +vi.mock("../webviewMessageHandler", () => ({ + webviewMessageHandler: vi.fn().mockResolvedValue(undefined), +})) + +type PersistedEntry = NonNullable[string] +type ViewStates = Record + +describe("ClineProvider - persisted view state write ordering", () => { + let provider: ClineProvider + let durable: Record + let failNextViewStatesWrite = false + let mockContext: vscode.ExtensionContext + // The durable profile value the storage double held at the instant the per-view write + // rejected, so a test can tell a shared write that reached storage from one that only + // moved the in-memory cache. + let durableProfileAtViewWriteFailure: unknown + let mockOutputChannel: vscode.OutputChannel + + // The tombstone set and the write queue are per-class, so every test starts from an + // empty ledger: otherwise a ledger entry from the previous test reads as a failure + // of this one. + const tombstones = () => ClineProvider["clearedViewStateIds"] + const evictions = () => provider["prunedViewIdsDuringActivation"] + // One cast: the durable double stores every key as unknown, and the only reader + // here is the viewStates map this suite seeds. + const viewStates = () => durable["viewStates"] as ViewStates | undefined + + beforeEach(() => { + vi.clearAllMocks() + failNextViewStatesWrite = false + durableProfileAtViewWriteFailure = undefined + + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + durable = { mode: "code", currentApiConfigName: "test-config", viewStates: {} } + + const secrets: Record = {} + + // Structural double providing only the storage members ClineProvider and ContextProxy + // read. The viewStates update rejects once per armed test, which is how a durable + // write failure is modelled: the in-memory cache is still updated first. + const globalState: Record = durable + const update = vi.fn((key: string, value: unknown) => { + if (key === "viewStates" && failNextViewStatesWrite) { + failNextViewStatesWrite = false + durableProfileAtViewWriteFailure = globalState["currentApiConfigName"] + return Promise.reject(new Error("durable write failed")) + } + globalState[key] = value + return Promise.resolve() + }) + mockContext = { + extensionPath: "/test/path", + extensionUri: { fsPath: "/test/path" }, + globalState: { + get: vi.fn((key: string) => globalState[key]), + update, + keys: vi.fn(() => Object.keys(globalState)), + }, + secrets: { + get: vi.fn((key: string) => secrets[key]), + store: vi.fn((key: string, value: string | undefined) => { + secrets[key] = value + return Promise.resolve() + }), + delete: vi.fn((key: string) => { + delete secrets[key] + return Promise.resolve() + }), + }, + workspaceState: { + get: vi.fn(() => undefined), + update: vi.fn(() => Promise.resolve()), + keys: vi.fn(() => []), + }, + subscriptions: [], + extension: { packageJSON: { version: "1.0.0" } }, + globalStorageUri: { fsPath: "/test/storage/path" }, + } as unknown as vscode.ExtensionContext + + // Structural double providing only the logging members ClineProvider calls. + mockOutputChannel = { + appendLine: vi.fn(), + clear: vi.fn(), + dispose: vi.fn(), + } as unknown as vscode.OutputChannel + + tombstones().clear() + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + + provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + }) + + afterEach(async () => { + await provider.dispose() + }) + + test("records no tombstone when the write that clears an entry rejects", async () => { + const viewStateId = provider["viewId"] + durable["viewStates"] = { + [viewStateId]: { mode: "code", currentApiConfigName: "profile-a", updatedAt: 1 }, + } + failNextViewStatesWrite = true + + await expect( + provider["savePersistedViewState"]({ mode: undefined, currentApiConfigName: undefined }), + ).rejects.toThrow("durable write failed") + + // The map on disk still holds the entry, so the tombstone that would tell a later + // activation to skip it was never earned. + expect(tombstones().has(viewStateId)).toBe(false) + expect(Object.hasOwn(viewStates() ?? {}, viewStateId)).toBe(true) + expect(evictions()).toEqual([]) + }) + + test("records the tombstone once the clearing write lands, and lifts it on the next landing write", async () => { + const viewStateId = provider["viewId"] + durable["viewStates"] = { + [viewStateId]: { mode: "code", currentApiConfigName: "profile-a", updatedAt: 1 }, + } + + await provider["savePersistedViewState"]({ mode: undefined, currentApiConfigName: undefined }) + + expect(tombstones().has(viewStateId)).toBe(true) + expect(Object.hasOwn(viewStates() ?? {}, viewStateId)).toBe(false) + + await provider["savePersistedViewState"]({ mode: "code", currentApiConfigName: "profile-a" }) + + expect(tombstones().has(viewStateId)).toBe(false) + expect(viewStates()?.[viewStateId]?.mode).toBe("code") + }) + + test("records evictions only for the map that reached durable storage", async () => { + // The cap keeps the 50 most recent closed-view entries, so 52 seeded entries evict + // the two oldest while the live view's own entry is never aged out. + const viewStateId = provider["viewId"] + const seeded: ViewStates = { + [viewStateId]: { mode: "code", currentApiConfigName: "profile-a", updatedAt: 100 }, + } + for (let index = 0; index < 52; index++) { + seeded[`closed-${index}`] = { mode: "code", currentApiConfigName: "profile-a", updatedAt: index } + } + durable["viewStates"] = seeded + failNextViewStatesWrite = true + + await expect(provider["savePersistedViewState"]({ currentApiConfigName: "profile-b" })).rejects.toThrow( + "durable write failed", + ) + + expect(evictions()).toEqual([]) + expect(tombstones().size).toBe(0) + + await provider["savePersistedViewState"]({ mode: "code", currentApiConfigName: "profile-b" }) + + expect(evictions()).toEqual(["closed-0", "closed-1"]) + expect(Object.hasOwn(viewStates() ?? {}, "closed-0")).toBe(false) + }) + + test("leaves an entry this view never wrote under its temporary id", async () => { + // Temporary ids restart from 0 on every host start, so this entry can belong to an + // earlier session: re-keying it would adopt another session's pinned profile. + const tempId = provider["viewId"] + durable["viewStates"] = { + [tempId]: { mode: "plan", currentApiConfigName: "profile-a", updatedAt: 5 }, + } + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + expect(Object.hasOwn(viewStates() ?? {}, tempId)).toBe(true) + expect(Object.hasOwn(viewStates() ?? {}, "stable-view")).toBe(false) + expect(tombstones().has(tempId)).toBe(false) + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(false) + }) + + test("re-keys an entry this view wrote under its temporary id", async () => { + const tempId = provider["viewId"] + + await provider["savePersistedViewState"]({ mode: "code", currentApiConfigName: "profile-a" }) + + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(true) + expect(Object.hasOwn(viewStates() ?? {}, tempId)).toBe(true) + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + expect(Object.hasOwn(viewStates() ?? {}, "stable-view")).toBe(true) + expect(Object.hasOwn(viewStates() ?? {}, tempId)).toBe(false) + expect(viewStates()?.["stable-view"]?.currentApiConfigName).toBe("profile-a") + expect(tombstones().has(tempId)).toBe(true) + expect(tombstones().has("stable-view")).toBe(false) + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(false) + }) + + test("records no tombstone when the re-key write rejects", async () => { + const tempId = provider["viewId"] + + await provider["savePersistedViewState"]({ mode: "code", currentApiConfigName: "profile-a" }) + failNextViewStatesWrite = true + + await expect(provider["rekeyPersistedViewStateEntry"]("stable-view")).rejects.toThrow("durable write failed") + + // The entry is still under the temporary name, so the next registration must be able + // to re-key it: recording the tombstone pair here would make the retry skip it. + expect(tombstones().size).toBe(0) + expect(Object.hasOwn(viewStates() ?? {}, tempId)).toBe(true) + expect(Object.hasOwn(viewStates() ?? {}, "stable-view")).toBe(false) + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(true) + }) + + // Every reject path in setViewStateId stops before a write, so the assertions are about the id + // this provider ends up holding and the map on disk, not about which log line it emitted. + const rejectedViewIds: { label: string; value: string | undefined }[] = [ + { label: "a non-string id", value: undefined }, + { label: "an Object.prototype member name", value: "__proto__" }, + { + label: "an id longer than the bound", + value: "v".repeat(ClineProvider["MAX_VIEW_STATE_ID_LENGTH"] + 6), + }, + ] + + it.each(rejectedViewIds)( + "refuses $label through the public setter and writes nothing durable", + async ({ value }) => { + const initialId = provider["viewStateId"] + + await provider.setViewStateId(value) + + expect(provider["viewStateId"]).toBe(initialId) + expect(viewStates()).toEqual({}) + expect(tombstones().size).toBe(0) + }, + ) + + test("refuses a view id another live view already claimed", async () => { + const initialId = provider["viewStateId"] + const other = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + other["viewStateId"] = "shared-view-id" + + try { + expect(ClineProvider.getAllInstances()).toContain(other) + + await provider.setViewStateId("shared-view-id") + + // Staying on the temporary id is the whole refusal: adopting the claimant's id would + // load that view's pinned profile into this provider and let this webview overwrite + // that view's persisted entry. + expect(provider["viewStateId"]).toBe(initialId) + expect(other["viewStateId"]).toBe("shared-view-id") + expect(viewStates()).toEqual({}) + } finally { + await other.dispose() + } + }) + + test("adopts a valid id through the public setter and re-keys the entry this view wrote", async () => { + const tempId = provider["viewId"] + const stableId = "11111111-1111-4111-8111-111111111111" + // Registering an id ends with a load, and that load reads the custom-modes registry. + // Stubbing the registry read keeps this test about adoption and re-keying rather than + // about what the mode file happens to contain. + vi.spyOn(provider.customModesManager, "getCustomModes").mockResolvedValue([]) + + await provider["savePersistedViewState"]({ mode: "code", currentApiConfigName: "profile-a" }) + + await provider.setViewStateId(stableId) + + expect(provider["viewStateId"]).toBe(stableId) + expect(viewStates()?.[stableId]?.currentApiConfigName).toBe("profile-a") + expect(Object.hasOwn(viewStates() ?? {}, tempId)).toBe(false) + expect(tombstones().has(tempId)).toBe(true) + expect(tombstones().has(stableId)).toBe(false) + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(false) + }) + + test("keeps a mode slug only while the custom-modes registry knows it", async () => { + const viewStateId = provider["viewId"] + const getCustomModes = vi.spyOn(provider.customModesManager, "getCustomModes").mockResolvedValue([]) + const customMode: ModeConfig = { + slug: "custom-review", + name: "Custom Review", + roleDefinition: "Review role", + groups: ["read"], + } + + // Captured rather than assumed: the proxy seeds its cache from the double's storage, and + // this suite only needs the dropped slug to leave the shared value where it started. + const before = provider.contextProxy.getValue("mode") + + await provider.setValues({ mode: "custom-review" }) + + // The mode gate runs before anything is forwarded, so a slug the registry does not know + // must reach neither the shared store nor this view's persisted entry. + expect(provider.contextProxy.getValue("mode")).toBe(before) + expect(viewStates()?.[viewStateId]?.mode).toBeUndefined() + + getCustomModes.mockResolvedValue([customMode]) + + await provider.setValues({ mode: "custom-review" }) + + expect(provider.contextProxy.getValue("mode")).toBe("custom-review") + expect(viewStates()?.[viewStateId]?.mode).toBe("custom-review") + }) + + test("restores the shared value and rethrows when the per-view write fails", async () => { + const viewStateId = provider["viewId"] + + await provider.setValues({ currentApiConfigName: "profile-a" }) + failNextViewStatesWrite = true + + await expect(provider.setValues({ currentApiConfigName: "profile-b" })).rejects.toThrow("durable write failed") + + // The shared store already carried the new name when the per-view record failed, so it is + // rolled back to the value this call started from instead of being left ahead of the + // durable per-view record. + expect(provider.contextProxy.getValue("currentApiConfigName")).toBe("profile-a") + expect(viewStates()?.[viewStateId]?.currentApiConfigName).toBe("profile-a") + }) + + test("leaves the shared profile unset and creates no entry when the first per-view write fails", async () => { + const viewStateId = provider["viewId"] + + // Cold state: this profile was never chosen, so there is no earlier value to roll back to. + // Unset through the same store the provider reads, so the proxy cache and storage agree + // that the key is unset before the call starts instead of merely disagreeing about it. + await provider.contextProxy.setValue("currentApiConfigName", undefined) + expect(provider.contextProxy.getValue("currentApiConfigName")).toBeUndefined() + + failNextViewStatesWrite = true + + await expect(provider.setValues({ currentApiConfigName: "profile-a" })).rejects.toThrow("durable write failed") + + // The shared write landed before the per-view write was attempted: the double held + // "profile-a" in storage at the instant of the failure, so what follows is a rollback of + // a value that really moved, not a no-op over a key that never left the cache. + expect(durableProfileAtViewWriteFailure).toBe("profile-a") + + // A rollback here has no earlier name to restore: the only correct outcomes are that the + // shared key stays unset in storage and not only in the cache this call reads back, that + // no per-view entry was created for the write that failed, that the view-local buffer + // never took the profile because it is advanced only after the durable write lands, and + // that the ledger stayed empty because nothing was ever cleared from storage. + expect(provider.contextProxy.getValue("currentApiConfigName")).toBeUndefined() + expect(durable["currentApiConfigName"]).toBeUndefined() + expect(Object.hasOwn(viewStates() ?? {}, viewStateId)).toBe(false) + expect(Object.hasOwn(provider["viewLocalState"], "currentApiConfigName")).toBe(false) + expect(tombstones().size).toBe(0) + }) +})