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..52ef624e56 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -113,6 +113,7 @@ export interface ExtensionMessage { | "rooHistoryImportProgress" | "themeFixtureProbeRequest" | "diagnosticsRequest" + | "viewStateClaim" text?: string /** For fileContent: { path, content, error? } */ fileContent?: { path: string; content: string | null; error?: string } @@ -171,6 +172,13 @@ export interface ExtensionMessage { // eslint-disable-next-line @typescript-eslint/no-explicit-any values?: Record requestId?: string + /** + * For `viewStateClaim`: the durable `viewStates` key the delivered claim governs. The + * message is only ever sent to the webview that key belongs to. + */ + viewStateId?: string + /** For `viewStateClaim`: the host-minted token the webview stores and echoes on launch. */ + viewStateClaim?: string themeFixture?: WebviewThemeFixture diagnostics?: WebviewDiagnosticsSnapshot promptText?: string @@ -712,6 +720,9 @@ export interface WebviewMessage { | "themeFixtureProbeResponse" | "diagnosticsResponse" text?: string + viewStateId?: string + /** For `webviewDidLaunch`: the claim delivered for `viewStateId`, enforced by setViewStateId. */ + viewStateClaim?: string taskId?: string editedMessageContent?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud" diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index abcd3c28a0..efc571867e 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -374,7 +374,7 @@ describe("extension.ts", () => { const getAll = vi.mocked(ClineProvider.getAllInstances) const post = vi.mocked(provider.postMessageToWebview) getAll.mockReturnValue([provider, provider]) - post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(undefined) + post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(true) try { const { activate } = await import("../extension") await activate(mockContext) diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index e53f60fb83..fb26bf9507 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -67,8 +67,21 @@ const globalSettingsExportSchema = globalSettingsSchema.omit({ taskHistory: true, listApiConfigMeta: true, currentApiConfigName: true, + // Host-minted claim tokens and per-view selections are not user-settings: exporting + // them would let a settings file be replayed as another view's identity. + viewStates: true, }) +/** + * Global settings a settings file may not carry in either direction. Per-view selections and the + * claim token that binds them are minted and held by the extension host, so a file that could + * set them would be replayable as another view's identity. The export side omits the key above; + * the import side consults this list in `sanitizeGlobalSettings`. + */ +const NON_PORTABLE_SETTINGS_KEYS: readonly string[] = ["viewStates"] + +export const isNonPortableSettingsKey = (key: string): boolean => NON_PORTABLE_SETTINGS_KEYS.includes(key) + export class ContextProxy { private readonly originalContext: vscode.ExtensionContext @@ -666,7 +679,16 @@ export class ContextProxy { * rollback, must provide it themselves. */ public async setValues(values: RooCodeSettings) { - const entries = Object.entries(values) as [RooCodeSettingsKey, unknown][] + // viewStates is host-owned: per-view selections and the claim token that binds them are + // minted by the extension host, so no configuration path may carry them. Every caller of + // this setter takes input that did not originate inside the host - a posted updateSettings, + // a newTask taskConfiguration, the external API, an imported settings file - so the filter + // lives here rather than at whichever caller a reviewer happened to point at. The host's + // own view-state methods write through setValue("viewStates", ...), which stays open. + const entries = Object.entries(values).filter(([key]) => !isNonPortableSettingsKey(key)) as [ + RooCodeSettingsKey, + unknown, + ][] const results = await Promise.allSettled(entries.map(([key, value]) => this.setValue(key, value))) const committed: string[] = [] 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..a4232c3a05 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -837,6 +837,25 @@ describe("ContextProxy", () => { }) }) + describe("export()", () => { + it("omits host-owned viewStates from a settings export", async () => { + await proxy.setValue("viewStates", { + "export-view": { claimToken: "export-claim-token", mode: "architect" }, + }) + // A portable global setting the export schema keeps: without it the test would pass on an + // empty export, because every remaining check only asserts absence. + await proxy.setValue("customInstructions", "retain these instructions") + // The cache carries the key; only the export must not. + expect(Object.keys(proxy.getValues())).toContain("viewStates") + const exported = await proxy.export() + expect(exported?.customInstructions).toBe("retain these instructions") + expect(exported && Object.keys(exported)).not.toContain("viewStates") + expect(exported && "viewStates" in exported).toBe(false) + // The value must not escape under another name either: the claim token is host state. + expect(JSON.stringify(exported)).not.toContain("export-claim-token") + }) + }) + describe("resetAllState", () => { it("should clear all in-memory caches", async () => { // Setup initial state in caches 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..4ea47d2298 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -251,6 +251,55 @@ describe("importExport", () => { }) }) + describe("host-owned view state on import", () => { + const settingsWithImportedViewStates = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "test", + apiConfigs: { + test: { apiProvider: providerIdentifiers.openai, apiKey: "test-key", id: "test-id" }, + }, + }, + globalSettings: { + customInstructions: "keep me", + viewStates: { "view-1": { mode: "architect", claimToken: "planted-claim" } }, + }, + }) + + beforeEach(() => { + ;(fs.readFile as Mock).mockResolvedValue(settingsWithImportedViewStates) + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: providerIdentifiers.anthropic, id: "default-id" } }, + }) + mockProviderSettingsManager.import.mockResolvedValue(undefined) + mockProviderSettingsManager.listConfig.mockResolvedValue([]) + mockContextProxy.setValues.mockResolvedValue(undefined) + mockContextProxy.setValue.mockResolvedValue(undefined) + mockContextProxy.setProviderSettings.mockResolvedValue(undefined) + }) + + it("drops viewStates so an imported file cannot plant another view's claim", async () => { + const result = await importSettingsFromPath("/mock/path/settings.json", { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + expect(result).toMatchObject({ success: true }) + + // A claim token is host-minted: a file that could carry an entry could replay it as that + // view's identity, so the key is dropped before anything is written. + expect(mockContextProxy.setValues).toHaveBeenCalledTimes(1) + const [appliedSettings] = (mockContextProxy.setValues as Mock).mock.calls[0] + expect(appliedSettings).not.toHaveProperty("viewStates") + expect(appliedSettings).toMatchObject({ customInstructions: "keep me" }) + const warnings = (result as { warnings?: string[] }).warnings ?? [] + expect(warnings).toHaveLength(1) + expect(warnings[0]).toContain('globalSettings.viewStates" was skipped') + expect(warnings[0]).toContain("not portable") + }) + }) + describe("importSettings", () => { it("should return success: false when user cancels file selection", async () => { ;(vscode.window.showOpenDialog as Mock).mockResolvedValue(undefined) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 9882039ad3..99f3ef6f6c 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -17,7 +17,7 @@ import { import { TelemetryService } from "@roo-code/telemetry" import { ProviderSettingsManager, providerProfilesSchema } from "./ProviderSettingsManager" -import { ContextProxy } from "./ContextProxy" +import { ContextProxy, isNonPortableSettingsKey } from "./ContextProxy" import { CustomModesManager } from "./CustomModesManager" import { downgradeLegacyRooConfig, ROUTER_REMOVAL_IMPORT_WARNING } from "./routerRemoval" import { resolveDefaultSaveUri, saveLastExportPath } from "../../utils/export" @@ -105,6 +105,11 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): { continue } + if (isNonPortableSettingsKey(key)) { + warnings.push(`Setting "${path}" was skipped: host-owned view state is not portable.`) + continue + } + let valueToValidate = rawValue if (key === "imageGenerationProvider" && rawValue === retiredProviderIdentifiers.roo) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 87c37cf596..5bbfc1b319 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,21 @@ import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" import { validateAndFixToolResultIds } from "../task/validateToolResultIds" import { PendingEditOperationStore, type PendingEditOperationInput } from "./PendingEditOperationStore" +type PersistedViewState = NonNullable[string] + +/** + * Values that can be held in a view-local state buffer (in-memory) and, for the + * non-secret subset, persisted durably per stable view id. + */ +type ViewLocalStateValues = Partial & Partial + +/** + * The subset of view-local values that has a durable per-view record. `saveViewState` + * and the mutation path handle exactly these keys, so the generic stays narrow enough + * for the compiler to reject e.g. `writeDelayMs`. + */ +type PersistedViewStateValues = Pick + /** * https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts * https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts @@ -237,6 +253,55 @@ function removeTaskEventListeners(listeners: WeakMap void>>, t listeners.delete(task) } } +/** + * How a host-minted view-state claim reached the webview that has to carry it back. + * + * - `delivered`: the launched webview accepted the post. + * - `queued`: the durable entry carries the token and the post is held for the next + * `webviewDidLaunch`, when the renderer has a listener that can receive it. + * - `dropped`: the provider was disposed, so nothing will post it again. + * - `none`: this operation minted no claim. + */ +export type ViewStateClaimDelivery = "delivered" | "queued" | "dropped" | "none" + +/** + * Result of a durable view-state write: whether anything was stored, and what happened to + * the claim the write minted. A save that stored an entry whose claim never reached the + * webview is a partial success, and `claim` is how the caller can see it. + */ +export type ViewStatePersistenceResult = { persisted: boolean; claim: ViewStateClaimDelivery } + +/** + * Result of registering a stable view id: whether the id was adopted, and what happened to + * the claim this registration minted or re-affirmed. + */ +export type ViewStateRegistrationResult = { adopted: boolean; claim: ViewStateClaimDelivery } + +/** + * One serialized view-state write. The queue holds entries like this instead of closures over a + * provider: `dispose()` clears `provider`, so a write still waiting behind another view's write + * releases the provider that scheduled it rather than retaining it, and its turn is skipped once + * it has been detached. + */ +type ViewStateWriteEntry = { + provider: ClineProvider | null + readonly generation: number + readonly run: (provider: ClineProvider) => Promise +} + +/** The fields a view owns in its buffer and in its durable record. */ +type ViewLocalStateField = "mode" | "currentApiConfigName" | "apiConfiguration" + +/** Writes one field of a partial record, or deletes it when the update is nullish. */ +function assignOrClearField(target: Partial, key: K, value: T[K] | null | undefined): void { + if (value === undefined || value === null) { + delete target[key] + + return + } + + target[key] = value +} export class ClineProvider extends EventEmitter @@ -248,6 +313,12 @@ export class ClineProvider public static readonly sideBarId = `${Package.name}.SidebarProvider` public static readonly tabPanelId = `${Package.name}.TabPanelProvider` private static activeInstances: Set = new Set() + private static nextViewId = 0 + private static readonly MAX_PERSISTED_VIEW_STATES = 50 + private static readonly MAX_VIEW_STATE_ID_LENGTH = 64 + /** Queued view-state writes, held as entries rather than as closures over a provider. */ + private static pendingViewStateWrites: ViewStateWriteEntry[] = [] + private static viewStateWritesPump: Promise = Promise.resolve() private webviewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< string, @@ -415,6 +486,74 @@ 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 + + /** + * Claims this view minted that its webview has not accepted yet, keyed by view id and holding + * the exact token the durable entry carries. A claim is host-owned: the webview can never present + * an identity the host did not hand it, so an undelivered claim is retried when a webview attaches + * instead of being dropped, and nothing re-mints it in the meantime. + */ + private pendingViewStateClaims = new Map() + + /** This provider's entries on the shared write queue; detached by `dispose()`. */ + private scheduledViewStateWrites: ViewStateWriteEntry[] = [] + + /** + * Bumped at the start of dispose(). View-state persistence is serialized on a + * static queue shared by every provider, so a closure this instance scheduled can + * start after the view is gone - behind another provider's write. Each closure + * captures the generation it was scheduled under and drops its own effects (the + * global-state write, provider-field mutation, claim delivery) once the value no + * longer matches, instead of applying them to a disposed provider and holding it + * retained until the queue advances. + */ + + private viewStateGeneration = 0 + + /** + * 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 = {} + + // Per-field write counters: a same-value save leaves the buffer value unchanged, so the + // load merge cannot detect it by comparing values alone. + private viewLocalStateWriteVersions = { mode: 0, currentApiConfigName: 0, apiConfiguration: 0 } + + /** + * View ids whose persisted selections failed to load. A load failure must not be + * indistinguishable from a successful one, because the unchanged-id guard in + * setViewStateId would then swallow every later registration of that id. A pending + * id is retried (claim check included) and removed from this set only after a load + * that ran to completion. + */ + private pendingViewStateRestorations = new Set() + + /** + * This view's pinned profile name from the view-local buffer. Exposed for + * sibling-instance inspection (getAllInstances() filtering): `viewLocalState` + * is private and must not be reached through bracket access. + */ + get pinnedProfileName(): string | undefined { + return this.viewLocalState.currentApiConfigName + } + public isViewLaunched = false public settingsImportedAt?: number public readonly latestAnnouncementId = "oct-2026-v3.94.0-models-oauth-chat" // v3.94.0 model support, MCP OAuth, and chat reliability @@ -430,13 +569,17 @@ 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, (message) => this.log(message), ) - ClineProvider.activeInstances.add(this) this.diagnosticsRecorder.record({ boundary: "provider", phase: "success", type: "created" }) this.mdmService = mdmService @@ -599,6 +742,555 @@ 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. Entries that are not + // objects are dropped as well: a corrupt or hand-edited `viewStates` record can + // hold a null or primitive value under a key, and prunePersistedViewStates reads + // `updatedAt` from every entry it is given - one such entry rejects the queued + // write and loses the selection this writer was about to store. + const kept = Object.entries(viewStates).filter(([, entry]) => entry && typeof entry === "object") + return Object.assign(Object.create(null), Object.fromEntries(kept)) + } + + /** Whether view-state work scheduled under `generation` may still apply effects. */ + private isViewStateGenerationLive(generation: number): boolean { + return !this._disposed && this.viewStateGeneration === generation + } + + /** + * Serialize one provider's view-state write behind every write already on the queue. The queue + * holds entries rather than closures over a provider, so `dispose()` can detach a write that is + * still waiting: its turn is skipped when the queue reaches it, and the provider is released + * instead of retained behind another view's write. + */ + private static enqueuePersistedViewStateWrite(entry: ViewStateWriteEntry): Promise { + ClineProvider.pendingViewStateWrites.push(entry) + entry.provider?.scheduledViewStateWrites.push(entry) + + const settled = ClineProvider.viewStateWritesPump.then(async () => { + const index = ClineProvider.pendingViewStateWrites.indexOf(entry) + + // Detached by dispose() while it waited: the view is gone, so its turn + // is skipped. + if (index === -1) return + + ClineProvider.pendingViewStateWrites.splice(index, 1) + + // The generation check lives in the queue step rather than in whichever + // writer a reviewer pointed at, so every entry on the queue - including one a + // future caller adds - is skipped the same way once the view it was scheduled + // for is gone. + const owner = entry.provider + + if (owner && owner.isViewStateGenerationLive(entry.generation)) { + try { + await entry.run(owner) + } finally { + // Detach the entry from the provider that scheduled it, whether this write + // resolved or rejected. Only this removal keeps a long-lived provider from + // holding every entry it ever scheduled, together with the run closure and + // the values that closure captured. + const scheduled = owner.scheduledViewStateWrites + const ownIndex = scheduled.indexOf(entry) + + if (ownIndex !== -1) scheduled.splice(ownIndex, 1) + } + } + }) + + // The queue advances even when this write rejects; the caller still sees the rejection. + ClineProvider.viewStateWritesPump = settled.catch(() => {}) + + return settled + } + + /** 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 + const generation = this.viewStateGeneration + // Set by the queued write when it had to mint a claim, delivered after it lands. + let mintedClaimToken: string | undefined + // Set once the queued write reaches the storage call: a write the queue dropped before + // its turn must not be reported as persisted. + let storedEntry = false + // Set when the durable write is undone because the view died before its claim could reach a + // renderer. + let rolledBackEntry = false + const run = async (provider: ClineProvider): Promise => { + const states = provider.getPersistedViewStates({ fresh: true }) + // Snapshot what this key held before the write, so a write interrupted by disposal can be + const previousEntry = states[viewStateId] + // 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 !== provider.viewId || provider.wroteUnderTemporaryViewStateId + const current = ownsTargetEntry ? (states[viewStateId] ?? {}) : {} + const next: PersistedViewState = { ...current } + + if ("mode" in values) assignOrClearField(next, "mode", values.mode) + + if ("currentApiConfigName" in values) + assignOrClearField(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 provider.contextProxy.setValue("viewStates", provider.prunePersistedViewStates(states)) + storedEntry = true + + if (!provider.isViewStateGenerationLive(generation)) { + // The view went away while the write was in flight: no provider field may be touched + // afterwards. If this write is the one that minted the entry's claim, no renderer was ever + // handed that token and the only provider that could post it is gone, so the durable + // entry would carry a binding nobody can ever present. Undo the write instead of leaving + // it: the record goes back to exactly what it held before this write. + if (mintedClaimToken) { + const after = provider.getPersistedViewStates({ fresh: true }) + // Undo only this write's binding: a later provider that already re-bound the key + // stored its own token, and that write must survive the rollback. + if (after[viewStateId]?.claimToken === mintedClaimToken) { + assignOrClearField(after, viewStateId, previousEntry) + + await provider.contextProxy.setValue("viewStates", provider.prunePersistedViewStates(after)) + rolledBackEntry = true + } + } + + return + } + + // 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 === provider.viewId) provider.wroteUnderTemporaryViewStateId = true + } + + await ClineProvider.enqueuePersistedViewStateWrite({ provider: this, generation, run }) + + if (!storedEntry || rolledBackEntry) { + // Either the queue dropped this write before it reached storage, or the durable + // write was undone because its claim could never reach a renderer: in both + // cases nothing was persisted, so nothing about a claim may be reported as + // settled either. + return { persisted: false, claim: "none" } + } + + // 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. + // A provider disposed in the meantime has no webview left to deliver it to. + if (mintedClaimToken && this.isViewStateGenerationLive(generation)) { + return { + persisted: true, + claim: await this.postViewStateClaimToWebview(viewStateId, mintedClaimToken), + } + } + + // The entry is durable and carries a token this webview has never been handed. Report + // that explicitly: a save that silently resolves while its claim is undelivered is the + // persistence-integrity gap this result exists to make visible. + return { persisted: true, claim: mintedClaimToken ? "dropped" : "none" } + } + + /** + * Removes the given view's entry from the registered viewStates map. + * Runs through the serialized write queue to avoid racing concurrent view-state writes. + */ + /** + * Enqueues one serialized view-state write for this view's current generation. The three callers + * below pass a fresh-read/mutate/write closure and the same generation, so this is the same call + * typed once rather than a new policy. + */ + private enqueueViewStateWrite(run: (provider: ClineProvider) => Promise): Promise { + return ClineProvider.enqueuePersistedViewStateWrite({ + provider: this, + generation: this.viewStateGeneration, + run, + }) + } + + private async clearPersistedViewState(viewStateId = this.viewStateId): Promise { + const run = async (provider: ClineProvider): Promise => { + const states = provider.getPersistedViewStates({ fresh: true }) + delete states[viewStateId] + await provider.contextProxy.setValue("viewStates", states) + } + + await this.enqueueViewStateWrite(run) + } + + /** 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 run = async (provider: ClineProvider): Promise => { + const states = provider.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 provider.contextProxy.setValue("viewStates", provider.prunePersistedViewStates(states)) + rebound = true + } + + await this.enqueueViewStateWrite(run) + + return rebound ? claimToken : undefined + } + + /** + * Delivers the host-minted claim that governs a durable viewStates key to this view's webview + * and reports whether the webview accepted it. The webview only stores the token and echoes it + * back on the next launch: it can never mint or choose a claim itself. + * + * Awaited rather than fired and forgotten: the entry already carries this claim once the write + * lands, so a view that never receives it would be missing an identity the host believes it + * holds. Posts are gated on `isViewLaunched`: the renderer attaches its message listener during + * `webviewDidLaunch`, and the Webview API allows an earlier message to be lost even when + * postMessage resolves true. Every non-delivery is reported - renderer not launched, the post + * rejected, or the webview resolving the post as not delivered - and the exact claim is queued + * for the next `webviewDidLaunch` instead of dropped. The post itself stays the single place + * that records the webview-out diagnostic. + */ + private async postViewStateClaimToWebview( + viewStateId: string, + claimToken: string, + ): Promise { + if (this._disposed) { + this.log( + `[viewStateClaim] Claim for ${viewStateId} dropped for view ${this.viewId}: the provider is disposed.`, + ) + return "dropped" + } + + // Every non-delivery keeps the exact token for the next webviewDidLaunch flush + // and reports the reason, so a lost post stays visible instead of silently + // dropping a claim the durable entry already carries. + const queueUndelivered = (reason: string): ViewStateClaimDelivery => { + this.pendingViewStateClaims.set(viewStateId, claimToken) + this.log(`[viewStateClaim] Claim for ${viewStateId} to view ${this.viewId}: ${reason}`) + + return "queued" + } + + if (!this.view || !this.isViewLaunched) { + // Held rather than posted. The renderer only attaches its message listener once the + // webview launches, and the Webview API permits a message posted before that to be lost + // while postMessage still resolves true - which would delete a claim the webview never + // saw. Queuing keeps the exact token for the webviewDidLaunch flush. + return queueUndelivered("not delivered: the webview has not launched yet") + } + + let delivered: boolean + try { + delivered = await this.postMessageToWebview({ + type: "viewStateClaim", + viewStateId, + viewStateClaim: claimToken, + }) + } catch (error) { + const reason = error instanceof Error ? error.message : String(error) + return queueUndelivered("delivery failed: " + reason) + } + + if (!delivered) { + // A hidden or closing webview resolves the post as not delivered: the claim stays queued so + // the same token reaches the webview once it can accept messages again. + return queueUndelivered("not delivered: the webview rejected the message") + } + + this.pendingViewStateClaims.delete(viewStateId) + return "delivered" + } + + /** + * Flushes the claims this view minted that no webview accepted yet. Called from the + * `webviewDidLaunch` handler, once the renderer has attached its message listener, and a no-op + * until then: posting before that can be lost. Each queued entry keeps the exact token the + * durable entry stores, so a webview that missed a delivery still gets the identity it has to + * present on the next launch; an entry that fails again is re-queued for the next launch. + */ + public async deliverPendingViewStateClaims(): Promise { + if (this._disposed || !this.view || !this.isViewLaunched || this.pendingViewStateClaims.size === 0) return + + const pending = [...this.pendingViewStateClaims.entries()] + this.pendingViewStateClaims.clear() + + for (const [viewStateId, claimToken] of pending) { + await this.postViewStateClaimToWebview(viewStateId, 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 run = async (provider: ClineProvider): Promise => { + // Captured inside the turn so the post-write check can tell a generation that + // moved while this write was in flight from the one the queue step verified. + const scheduledGeneration = provider.viewStateGeneration + + // 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 (!provider.wroteUnderTemporaryViewStateId) return + + const states = provider.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 provider.contextProxy.setValue("viewStates", provider.prunePersistedViewStates(states)) + + if (!provider.isViewStateGenerationLive(scheduledGeneration)) { + // The move committed while the view it was written for went away. Leaving it applied + // binds the entry to a claim no renderer will ever present: the selection is stranded + // under a dead view id and the temporary name is already deleted. Put the entry back + // under the name its author can still find it by, and keep the authorship flag so the + // next launch re-keys it. The restore is best effort - if that write fails too the + // entry stays under the stable id rather than disappearing. + if (existingStableEntry) { + // Merged into a stable entry it cannot be pulled back out whole, so the temporary + // name is gone either way; authorship must not survive it, or every later launch + // keeps treating this view as the author of an entry that no longer exists. + provider.wroteUnderTemporaryViewStateId = false + return + } + const after = provider.getPersistedViewStates({ fresh: true }) + const moved = after[nextViewStateId] + if (moved) { + delete after[nextViewStateId] + after[previousViewStateId] = moved + try { + await provider.contextProxy.setValue("viewStates", provider.prunePersistedViewStates(after)) + } catch { + // The comment above promises best effort for the storage write only. If that write fails + // the entry is still under the stable id in storage, so the in-memory re-key above has to be + // undone as well: otherwise memory claims the temporary name is gone while storage still + // holds it, and authorship is cleared below even though a later launch still has to re-key + // the entry. Undo the re-key and keep the claim so the next launch can try again. + delete after[previousViewStateId] + after[nextViewStateId] = moved + } + } + return + } + + // 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. + provider.wroteUnderTemporaryViewStateId = false + } + + await this.enqueueViewStateWrite(run) + } + + /** Register the stable view id and load its persisted selections; a claim must match. */ + public async setViewStateId( + viewStateId: string | undefined, + presentedClaim?: string, + ): Promise { + // A refused registration reports why and adopts nothing, and its claim field says + // explicitly that no claim came out of it. + const refuseRegistration = (detail: string): ViewStateRegistrationResult => { + this.log(`[setViewStateId] ${detail}`) + + return { adopted: false, claim: "none" } + } + // 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. + return refuseRegistration(`Ignoring a non-string viewStateId for view ${this.viewId}.`) + } + + const normalizedViewStateId = viewStateId.trim().replace(/[^A-Za-z0-9_-]/g, "_") + + // A load that failed leaves the id adopted (the re-key already moved the entry to the + // stable name) with the selections unrestored, so the unchanged-id guard must not + // swallow the retry: while a restoration is pending for this exact id the + // registration continues, and the claim check below still has to pass. + const restorationPending = this.pendingViewStateRestorations.has(normalizedViewStateId) + + if ( + !normalizedViewStateId || + (normalizedViewStateId === this.viewStateId && !restorationPending) || + // 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 { adopted: false, claim: "none" } + } + + // 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) { + return refuseRegistration( + `Ignoring an oversized viewStateId (${normalizedViewStateId.length} chars) for view ${this.viewId}.`, + ) + } + + 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. + return refuseRegistration( + `Refusing ${normalizedViewStateId} for view ${this.viewId}: already claimed by live view ${claimant.viewId}.`, + ) + } + + // 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] + + // What became of the claim this registration mints or re-affirms. A refused registration + // never reaches a claim, and an adopted one whose claim is still queued is not the same + // outcome as one the webview received, so both are reported to the caller. + let claimDelivery: ViewStateClaimDelivery = "none" + + 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. + return refuseRegistration( + `Refusing ${normalizedViewStateId} for view ${this.viewId}: bound concurrently by another registration.`, + ) + } + + claimDelivery = await 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. + return refuseRegistration( + `Refusing ${normalizedViewStateId} for view ${this.viewId}: claim missing or mismatched.`, + ) + } + } + + const previousViewStateId = this.viewStateId + const generation = this.viewStateGeneration + + 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) + + // 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 + + // A registration whose view was disposed mid-flight must not deliver a claim to + // a webview that is no longer there. + if (adoptedClaim && this.isViewStateGenerationLive(generation)) { + claimDelivery = await this.postViewStateClaimToWebview(this.viewStateId, adoptedClaim) + } else if (adoptedClaim) { + // The view was disposed mid-registration: the binding is durable but this provider + // will not post the claim again. + claimDelivery = "dropped" + } + + // Registration is what loads this view's own persisted selections. + 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 + } + + return { adopted: true, claim: claimDelivery } + } + /** * Override EventEmitter's on method to match TaskProviderLike interface */ @@ -905,6 +1597,11 @@ export class ClineProvider private clearWebviewResources() { this.rejectPendingThemeFixtureProbes(new Error("Webview was disposed before the theme fixture probe completed")) this.stopWebviewWatchdog() + // The renderer behind this view is gone or about to be replaced, and its message + // listener dies with it: a post can still resolve true while nobody is there to receive + // it. The flag goes down here so a claim minted before the replacement launches is held + // in the queue and flushed by that replacement's own `webviewDidLaunch`. + this.isViewLaunched = false // Invalidate any recovery reload still awaiting its HTML so it cannot // reassign webview.html on the disposed view. this.webviewRecoveryEpoch++ @@ -944,13 +1641,30 @@ export class ClineProvider ) } } - async dispose() { if (this._disposed) { return } + // Invalidate view-state work that has not reached the queue head yet: it is + // serialized on the static queue with other providers' writes, so it can start + // after this provider is gone. + this.viewStateGeneration++ this._disposed = true + + // Detach this provider's queued writes. The queue holds entries rather than closures over + // this instance, so clearing the back-pointer releases the provider instead of keeping it + // retained behind a write that is still running for another view. + for (const entry of this.scheduledViewStateWrites) { + const index = ClineProvider.pendingViewStateWrites.indexOf(entry) + + if (index !== -1) ClineProvider.pendingViewStateWrites.splice(index, 1) + + entry.provider = null + } + this.scheduledViewStateWrites = [] + // A claim that never reached a webview dies with this view: nothing may re-post it later. + this.pendingViewStateClaims.clear() this._postStateToWebviewThrottled.cancel() this.stopWebviewWatchdog() this.diagnosticsRecorder.record({ boundary: "provider", phase: "start", type: "dispose" }) @@ -1195,6 +1909,11 @@ export class ClineProvider this.webviewDisposables, ) + // Queued claims are deliberately not posted here. The renderer attaches its message listener + // only when it launches, and the Webview API allows a message posted before that to be lost + // while postMessage still resolves true. The webviewDidLaunch handler flushes the queue once + // a listener exists to receive it. + if (inTabMode) { setPanel(webviewView, "tab") } else if ("onDidChangeVisibility" in webviewView) { @@ -1452,7 +2171,6 @@ export class ClineProvider ) return run } - private async createTaskWithHistoryItemUnlocked( historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }, options?: { startTask?: boolean }, @@ -1698,8 +2416,14 @@ export class ClineProvider return task } - - public async postMessageToWebview(message: ExtensionMessage) { + /** + * Posts a message to this view's webview and reports whether the webview accepted it. + * + * Callers that only fire a notification may ignore the result. Callers that act on delivery must + * await it: `postMessage` resolves false when the webview is hidden or its panel is gone, and a + * void return cannot tell a delivered message from a dropped one. + */ + public async postMessageToWebview(message: ExtensionMessage): Promise { const startedAt = Date.now() const taskId = this.getCurrentTask()?.taskId this.diagnosticsRecorder.record({ @@ -1719,7 +2443,7 @@ export class ClineProvider elapsedMs: Date.now() - startedAt, taskId, }) - return + return false } try { @@ -1733,6 +2457,8 @@ export class ClineProvider stateSequence: message.type === "state" ? message.state?.clineMessagesSeq : undefined, taskId, }) + + return delivered === true } catch { this.diagnosticsRecorder.record({ boundary: "webview-out", @@ -1742,8 +2468,11 @@ export class ClineProvider elapsedMs: Date.now() - startedAt, taskId, }) - // View disposed, drop message silently + // View disposed, or the webview rejected the message: callers that act on delivery + // need the failure, so it is reported instead of swallowed. } + + return false } public requestWebviewThemeFixture(timeoutMs = 5_000): Promise { @@ -1805,10 +2534,9 @@ export class ClineProvider public requestWebviewDiagnostics(timeoutMs = 1_000): Promise { if (this._disposed || !this.view) return Promise.resolve(undefined) - return this.diagnosticsRequestBroker.request( - (requestId) => this.postMessageToWebview({ type: "diagnosticsRequest", requestId }), - timeoutMs, - ) + return this.diagnosticsRequestBroker.request(async (requestId) => { + await this.postMessageToWebview({ type: "diagnosticsRequest", requestId }) + }, timeoutMs) } private async getHMRHtmlContent(webview: vscode.Webview): Promise { @@ -2048,7 +2776,6 @@ export class ClineProvider this.handleModeSwitchUnlocked(newMode, targetTask, signal), ) } - private async handleModeSwitchUnlocked( newMode: Mode, targetTask: Task | null | undefined, @@ -2322,7 +3049,6 @@ export class ClineProvider ) } } - private async persistProviderProfile( name: string, providerSettings: ProviderSettings, @@ -2383,7 +3109,6 @@ export class ClineProvider await this.postStateToWebview() return id } - async deleteProviderProfile(profileToDelete: ProviderSettingsEntry) { const globalSettings = this.contextProxy.getValues() let profileToActivate: string | undefined = globalSettings.currentApiConfigName @@ -2451,7 +3176,6 @@ export class ClineProvider this.activateProviderProfileUnlocked(args, options, signal), ) } - private async activateProviderProfileUnlocked( args: { name: string } | { id: string }, options?: { @@ -2583,9 +3307,6 @@ export class ClineProvider await this.upsertProviderProfile(currentApiConfigName, newConfiguration) } - - // Zoo Code Auth - async handleZooCodeCallback(token: string) { // Auth mutation (token storage, subscription check, success toast) was already // performed by handleAuthCallback() in handleUri.ts before this method was called. @@ -3515,8 +4236,18 @@ export class ClineProvider const taskSyncEnabled: boolean = false // Return the same structure as before. + // The three fields a view owns come from this view's own buffer first: the + // restored record's values and this view's edits belong to it, and the shared + // settings are the fallback for a field this view does not hold. apiConfiguration + // merges on top of the shared settings because a view's buffer can hold a partial + // profile update rather than a whole profile. + const viewLocalState = this.viewLocalState + const viewLocalApiConfiguration = viewLocalState.apiConfiguration + ? { ...providerSettings, ...viewLocalState.apiConfiguration } + : providerSettings + return { - apiConfiguration: providerSettings, + apiConfiguration: viewLocalApiConfiguration, lastShownAnnouncementId: stateValues.lastShownAnnouncementId, customInstructions: stateValues.customInstructions, apiModelId: stateValues.apiModelId, @@ -3565,11 +4296,11 @@ export class ClineProvider terminalZshP10k: stateValues.terminalZshP10k ?? false, terminalZdotdir: stateValues.terminalZdotdir ?? false, terminalProfile: stateValues.terminalProfile, - mode: stateValues.mode ?? defaultModeSlug, + mode: viewLocalState.mode ?? stateValues.mode ?? defaultModeSlug, language: stateValues.language ?? formatLanguage(vscode.env.language), mcpEnabled: stateValues.mcpEnabled ?? true, mcpServers: this.mcpHub?.getAllServers() ?? [], - currentApiConfigName: stateValues.currentApiConfigName ?? "default", + currentApiConfigName: viewLocalState.currentApiConfigName ?? stateValues.currentApiConfigName ?? "default", listApiConfigMeta: stateValues.listApiConfigMeta ?? [], pinnedApiConfigs: stateValues.pinnedApiConfigs ?? {}, modeApiConfigs: stateValues.modeApiConfigs ?? ({} as Record), @@ -3705,7 +4436,6 @@ export class ClineProvider private getGlobalState(key: K) { return this.contextProxy.getValue(key) } - public async setValue(key: K, value: RooCodeSettings[K]) { await this.contextProxy.setValue(key, value) } @@ -3713,19 +4443,14 @@ export class ClineProvider public getValue(key: K) { return this.contextProxy.getValue(key) } - public getValues() { return this.contextProxy.getValues() } - public async setValues(values: RooCodeSettings) { await this.contextProxy.setValues( withoutInvalidDisplaySettings(values, (key) => this.log(`[setValues] Ignored an invalid value for ${key}`)), ) } - - // dev - async resetState() { const answer = await vscode.window.showInformationMessage( t("common:confirmation.reset_state"), @@ -3957,6 +4682,10 @@ export class ClineProvider ) { return } + // Assigning new HTML discards the live document and its message listener, so the launched + // flag goes down with it; the regenerated renderer re-arms it from its own + // `webviewDidLaunch`, and any claim queued in between reaches it then. + this.isViewLaunched = false view.webview.html = html this.resetWebviewHeartbeatGrace() const revision = this.webviewHeartbeatRevision @@ -5487,4 +6216,184 @@ export class ClineProvider return vscode.Uri.file(filePath).toString() } } + + /** + * 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. + */ + public async saveViewState( + key: K, + value: PersistedViewStateValues[K] | undefined, + ): Promise { + const persistence = await this._saveViewLocalStateFromMutation({ [key]: value } as ViewLocalStateValues) + + this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`) + + return persistence + } + + /** + * 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 { + const persistence = await this._persistViewLocalStateFromMutation(values) + this._updateViewLocalStateFromMutation(values) + + return persistence + } + + /** + * 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 bumpViewLocalStateWriteVersions(values: Partial & Partial): void { + if ("mode" in values) this.viewLocalStateWriteVersions.mode += 1 + if ("currentApiConfigName" in values) this.viewLocalStateWriteVersions.currentApiConfigName += 1 + if ("apiConfiguration" in values) this.viewLocalStateWriteVersions.apiConfiguration += 1 + } + + private _updateViewLocalStateFromMutation(values: Partial & Partial): void { + this.bumpViewLocalStateWriteVersions(values) + if ("mode" in values) assignOrClearField(this.viewLocalState, "mode", values.mode) + + if ("currentApiConfigName" in values) + assignOrClearField(this.viewLocalState, "currentApiConfigName", values.currentApiConfigName) + + if ("apiConfiguration" in values) { + assignOrClearField(this.viewLocalState, "apiConfiguration", values.apiConfiguration) + } 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, + } + } + } + + /** + * 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) { + return this.savePersistedViewState(persistedValues) + } + + // Nothing in this mutation belongs to the durable record, so there was nothing to write. + return { persisted: false, claim: "none" } + } + + /** Load non-secret persisted selections; in-flight buffer edits are reapplied on top. */ + private async loadViewState(): Promise { + // 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 = 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 preLoadVersions = { ...this.viewLocalStateWriteVersions } + + 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 + } + + // 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 } + + const reapplyViewLocalField = (key: K): void => { + if (preLoadVersions[key] !== this.viewLocalStateWriteVersions[key]) { + const buffered = postLoadBuffer[key] + + if (buffered === undefined) { + delete mergedState[key] + } else { + mergedState[key] = buffered + } + } + } + + reapplyViewLocalField("mode") + reapplyViewLocalField("currentApiConfigName") + reapplyViewLocalField("apiConfiguration") + + this.viewLocalState = mergedState + this.pendingViewStateRestorations.delete(loadedForViewId) + this.log(`[loadViewState] Loaded state for viewId ${this.viewId}`) + } catch (error) { + // The load aborted, and `setViewStateId` keeps the id it adopted because the + // entry is already stored under the stable name. Record that this id is unrestored + // so the next registration of the same id is not swallowed by the unchanged-id + // guard; a completed load above clears it. + if (this.viewStateId === loadedForViewId) { + this.pendingViewStateRestorations.add(loadedForViewId) + } + this.log( + `[loadViewState] Error loading state for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 9826fc206b..e5627d4b46 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -15,6 +15,7 @@ import { type ClineMessage, type ExtensionMessage, type ExtensionState, + type GlobalState, type RooCodeSettings, type WebviewMessage, ORGANIZATION_ALLOW_ALL, @@ -3229,8 +3230,9 @@ describe("ClineProvider", () => { const message: ExtensionMessage = { type: "action", action: "chatButtonClicked" } - // Should not throw - await expect(provider.postMessageToWebview(message)).resolves.toBeUndefined() + // Should not throw, and must report the failed delivery rather than resolving as if the + // webview had taken the message. + await expect(provider.postMessageToWebview(message)).resolves.toBe(false) }) test("postMessageToWebview leaves originalContent of file-edit tool messages out of the state it posts", async () => { @@ -3315,7 +3317,7 @@ describe("ClineProvider", () => { }) test("posts a request and resolves the matching response", async () => { - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) const request = provider.requestWebviewThemeFixture() await Promise.resolve() const requestId = postMessageSpy.mock.calls[0]?.[0].requestId @@ -3331,7 +3333,7 @@ describe("ClineProvider", () => { test("rejects a request after its timeout", async () => { vi.useFakeTimers() - vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) const request = provider.requestWebviewThemeFixture(100) const rejection = expect(request).rejects.toThrow("Theme fixture probe timed out after 100ms") @@ -3340,7 +3342,7 @@ describe("ClineProvider", () => { }) test("rejects pending requests when webview resources are cleared", async () => { - vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) const request = provider.requestWebviewThemeFixture() const rejection = expect(request).rejects.toThrow( "Webview was disposed before the theme fixture probe completed", @@ -3366,7 +3368,7 @@ describe("ClineProvider", () => { mdmService, ) - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockImplementation(async () => undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockImplementation(async () => true) vi.spyOn(provider as any, "getStateToPostToWebview").mockResolvedValue({ version: "1.0.0" }) await provider.postStateToWebview() @@ -3376,8 +3378,8 @@ describe("ClineProvider", () => { }) test("postStateToWebviewWithoutTaskHistory waits for the webview post boundary", async () => { - let releasePost!: () => void - const pendingPost = new Promise((resolve) => { + let releasePost!: (delivered: boolean) => void + const pendingPost = new Promise((resolve) => { releasePost = resolve }) let statePostSettled = false @@ -3396,7 +3398,7 @@ describe("ClineProvider", () => { expect(postMessageSpy).toHaveBeenCalledOnce() expect(statePostSettled).toBe(false) - releasePost() + releasePost(true) await statePost expect(statePostSettled).toBe(true) }) @@ -3423,7 +3425,7 @@ describe("ClineProvider", () => { vi.spyOn(provider, "getStateToPostToWebview") .mockReturnValueOnce(olderSnapshot) .mockResolvedValueOnce(readyState) - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) const olderPost = postState(provider) await Promise.resolve() @@ -3450,7 +3452,7 @@ describe("ClineProvider", () => { ], ])("%s skips task history computation", async (_methodName, postState) => { const getAllSpy = vi.spyOn(provider.taskHistoryStore, "getAll") - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) await postState(provider) @@ -5266,7 +5268,6 @@ describe("ClineProvider", () => { // Spy on log method to verify warning was logged const logSpy = vi.spyOn(provider, "log") - // Create history item with non-existent mode const historyItem = { id: "test-id", @@ -5719,6 +5720,1626 @@ 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 nothing is persisted under it. + expect(provider["viewStateId"]).toBe(provider.viewId) + expect(mockContext.globalState.get("viewStates")).toBeUndefined() + expect(provider["viewLocalState"]).toEqual({}) + + await provider.dispose() + }) + + it("rejects a non-string viewStateId from a crafted message and stays on the temporary id", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // The message type declares a string, but a crafted postMessage can carry any JSON + // value, so the double assertion here is the crafted payload, not the call site. + const crafted = JSON.parse('{"viewStateId": 42}') as { viewStateId: string } + const statesBefore = mockContext.globalState.get("viewStates") + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + const loggedBefore = (mockOutputChannel.appendLine as ReturnType).mock.calls.length + + await provider["setViewStateId"](crafted.viewStateId) + + expect(provider["viewStateId"]).toBe(provider.viewId) + // Nothing registered means nothing persisted, nothing loaded, and no claim posted. + expect(mockContext.globalState.get("viewStates")).toBe(statesBefore) + expect(provider["viewLocalState"]).toEqual({}) + expect( + postMessageSpy.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + const loggedLines = (mockOutputChannel.appendLine as ReturnType).mock.calls + .slice(loggedBefore) + .map((call: unknown[]) => String(call[0])) + expect( + loggedLines.some((line) => line.includes("[setViewStateId] Ignoring a non-string viewStateId")), + ).toBe(true) + + postMessageSpy.mockRestore() + await provider.dispose() + }) + + it("rejects an empty viewStateId and keeps the temporary viewId", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const statesBefore = mockContext.globalState.get("viewStates") + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + + await provider["setViewStateId"]("") + + // The empty id returns without a log line; the absence of every side effect is the + // evidence that registration never happened. + expect(provider["viewStateId"]).toBe(provider.viewId) + // Nothing registered means nothing persisted, nothing loaded, and no claim posted. + expect(mockContext.globalState.get("viewStates")).toBe(statesBefore) + expect(provider["viewLocalState"]).toEqual({}) + expect( + postMessageSpy.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + postMessageSpy.mockRestore() + await provider.dispose() + }) + + it("rejects a whitespace-only viewStateId that normalizes to empty", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const statesBefore = mockContext.globalState.get("viewStates") + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + + // Whitespace normalizes to the empty string, which is the same rejection. + await provider["setViewStateId"](" ") + + expect(provider["viewStateId"]).toBe(provider.viewId) + // Nothing registered means nothing persisted, nothing loaded, and no claim posted. + expect(mockContext.globalState.get("viewStates")).toBe(statesBefore) + expect(provider["viewLocalState"]).toEqual({}) + expect( + postMessageSpy.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + postMessageSpy.mockRestore() + await provider.dispose() + }) + + it("rejects a viewStateId longer than the bound and logs the length", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const statesBefore = mockContext.globalState.get("viewStates") + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + const loggedBefore = (mockOutputChannel.appendLine as ReturnType).mock.calls.length + + // The webview generates a UUID; a 65-character value is not a view id and an + // unbounded key is unbounded growth in the shared viewStates map. + await provider["setViewStateId"]("v".repeat(65)) + + expect(provider["viewStateId"]).toBe(provider.viewId) + // Nothing registered means nothing persisted, nothing loaded, and no claim posted. + expect(mockContext.globalState.get("viewStates")).toBe(statesBefore) + expect(provider["viewLocalState"]).toEqual({}) + expect( + postMessageSpy.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + const loggedLines = (mockOutputChannel.appendLine as ReturnType).mock.calls + .slice(loggedBefore) + .map((call: unknown[]) => String(call[0])) + expect(loggedLines.some((line) => line.includes("Ignoring an oversized viewStateId (65 chars)"))).toBe(true) + expect(mockContext.globalState.get("viewStates")).toBe(statesBefore) + + postMessageSpy.mockRestore() + await provider.dispose() + }) + + it("loads the persisted selections of the registered view", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { + "stable-view": { claimToken: "host-token", mode: "architect", currentApiConfigName: "cfg-a" }, + }) + + await provider["setViewStateId"]("stable-view", "host-token") + + // Registration is what loads a view's own selections. + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider["viewLocalState"].currentApiConfigName).toBe("cfg-a") + + await provider.dispose() + }) + it("normalises a registered viewStateId and keys the adopted entry by it", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // The key the host may store, plus a decoy under the raw name a normalizer never + // produces: adopting that entry would load another view's selections. + await provider.contextProxy.setValue("viewStates", { + norm_view: { claimToken: "norm-token", mode: "architect", currentApiConfigName: "cfg-norm" }, + "norm/view": { claimToken: "decoy-token", mode: "code", currentApiConfigName: "cfg-decoy" }, + "norm/view ": { claimToken: "decoy2-token", mode: "ask", currentApiConfigName: "cfg-decoy2" }, + }) + const result = await provider["setViewStateId"](" norm/view ", "norm-token") + expect(result.adopted).toBe(true) + expect(provider["viewStateId"]).toBe("norm_view") + // The selections that came through are the normalized key's, not the decoy's. + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider["viewLocalState"].currentApiConfigName).toBe("cfg-norm") + const states = provider["getPersistedViewStates"]() + // The durable entry the registration read and adopted is the normalized one, and + // nothing was ever stored under the padded name the webview actually sent. + expect(states["norm_view"].claimToken).toBe("norm-token") + expect(Object.keys(states)).not.toContain(" norm/view ") + // The decoys sit under names a normalizer never produces: adopted, they would load + // another view's selections; here they stay exactly as seeded. + expect(states["norm/view"].claimToken).toBe("decoy-token") + expect(states["norm/view"].mode).toBe("code") + expect(states["norm/view "].claimToken).toBe("decoy2-token") + await provider.dispose() + }) + + it("resolves to true when the webview accepts the message", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.resolveWebviewView({ ...mockWebviewView, webview: { ...mockWebviewView.webview, html: "" } }) + // The spy replaces the resolve-time double so only this post is under test. + const postMessage = vi.spyOn(provider["view"]!.webview, "postMessage").mockResolvedValue(true) + await expect(provider.postMessageToWebview({ type: "action", action: "focusInput" })).resolves.toBe(true) + expect(postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + // The delivery result must reach the caller: a caller that rolls back on a failed + // post needs the true case to be true, not merely non-throwing. + await provider.dispose() + }) + + it("resolves to false when the webview refuses the delivery", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.resolveWebviewView({ ...mockWebviewView, webview: { ...mockWebviewView.webview, html: "" } }) + const postMessage = vi.spyOn(provider["view"]!.webview, "postMessage").mockResolvedValue(false) + await expect(provider.postMessageToWebview({ type: "action", action: "focusInput" })).resolves.toBe(false) + expect(postMessage).toHaveBeenCalledWith({ type: "action", action: "focusInput" }) + 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 through the real write path so the re-key has work to do. + 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. + expect(provider["viewStateId"]).toBe(provider.viewId) + + // A later retry succeeds once the storage write works again. + 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("persisted viewStates record hygiene", () => { + it("drops non-object entries so a corrupt record cannot reject a save", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // Test data only, hence the assertion: a null or primitive entry is exactly what the + // persisted schema forbids and what a half-written or hand-edited record can still hold. + const corruptStates = { + "corrupt-view": null, + "primitive-view": "not-an-entry", + "healthy-view": { claimToken: "host-token", mode: "code", updatedAt: 5 }, + } as unknown as GlobalState["viewStates"] + await provider.contextProxy.setValue("viewStates", corruptStates) + + // Both read paths filter: the ContextProxy-cached copy (loadViewState reads it) and the + // fresh storage copy every serialized writer reads. + expect(provider["getPersistedViewStates"]()).not.toHaveProperty("corrupt-view") + expect(provider["getPersistedViewStates"]()).not.toHaveProperty("primitive-view") + expect(provider["getPersistedViewStates"]({ fresh: true })["corrupt-view"]).toBeUndefined() + expect(provider["getPersistedViewStates"]({ fresh: true })["healthy-view"]).toMatchObject({ + claimToken: "host-token", + mode: "code", + }) + + // Before the filter this rejects: prunePersistedViewStates sorts every entry by + // `updatedAt`, and reading it from a null entry throws inside the queued write. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 10 }) + + const stored = mockContext.globalState.get>("viewStates") ?? {} + expect(stored[provider["viewStateId"]]).toMatchObject({ mode: "architect" }) + expect(stored).not.toHaveProperty("corrupt-view") + expect(stored).not.toHaveProperty("primitive-view") + + await provider.dispose() + }) + + it("deletes the persisted mode when a view-local save clears it", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const stored = () => + mockContext.globalState.get>( + "viewStates", + ) ?? {} + + await provider.saveViewState("mode", "architect") + await provider.saveViewState("currentApiConfigName", "cfg-a") + expect(stored()[provider["viewStateId"]]).toMatchObject({ + mode: "architect", + currentApiConfigName: "cfg-a", + }) + + const cleared = await provider.saveViewState("mode", undefined) + expect(cleared).toEqual({ persisted: true, claim: "none" }) + expect(stored()[provider["viewStateId"]]).not.toHaveProperty("mode") + expect(stored()[provider["viewStateId"]]).toMatchObject({ currentApiConfigName: "cfg-a" }) + expect(provider["viewLocalState"]).not.toHaveProperty("mode") + expect(provider["viewLocalState"].currentApiConfigName).toBe("cfg-a") + + await provider.dispose() + }) + + it("removes the record when the last persisted field is cleared", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const stored = () => + mockContext.globalState.get>( + "viewStates", + ) ?? {} + + await provider.saveViewState("mode", "architect") + await provider.saveViewState("currentApiConfigName", "cfg-a") + expect(stored()).toHaveProperty(provider["viewStateId"]) + + await provider.saveViewState("mode", undefined) + const cleared = await provider.saveViewState("currentApiConfigName", undefined) + expect(cleared).toEqual({ persisted: true, claim: "none" }) + // Deleting the last field deletes the entry rather than leaving an empty shell behind. + expect(stored()).not.toHaveProperty(provider["viewStateId"]) + expect(provider["viewLocalState"]).toStrictEqual({}) + + await provider.dispose() + }) + it("evicts exactly the oldest entry when a save crosses the persisted-view cap", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const cap = ClineProvider["MAX_PERSISTED_VIEW_STATES"] + expect(cap).toBe(50) + + // One entry over the cap, each with its own timestamp so the eviction order is forced + // rather than left to insertion order. + const seeded: Record = {} + + for (let i = 0; i < cap; i++) { + seeded["seed-" + i] = { claimToken: "token-" + i, mode: "code", updatedAt: i + 1 } + } + + await provider.contextProxy.setValue("viewStates", seeded as GlobalState["viewStates"]) + + const saved = await provider["savePersistedViewState"]({ mode: "architect", updatedAt: cap + 1 }) + expect(saved).toEqual({ persisted: true, claim: "queued" }) + + const stored = + mockContext.globalState.get>("viewStates") ?? {} + expect(Object.keys(stored)).toHaveLength(cap) + expect(stored[provider["viewStateId"]]).toMatchObject({ mode: "architect", updatedAt: cap + 1 }) + // The oldest entry is the one that goes. A cap that evicted an arbitrary survivor + // would satisfy the count above just as easily. + expect(stored).not.toHaveProperty("seed-0") + expect(stored["seed-1"]).toMatchObject({ updatedAt: 2 }) + expect(stored["seed-" + (cap - 1)]).toMatchObject({ updatedAt: cap }) + // And the write that triggered the eviction is never the entry evicted by it. + expect(Object.keys(stored).filter((key) => key.startsWith("seed-"))).toHaveLength(cap - 1) + + await provider.dispose() + }) + }) + + describe("view state restoration retry", () => { + const createProvider = () => + new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + it("reloads the selections of a view whose load failed when the same id is registered again", async () => { + const provider = createProvider() + await provider.contextProxy.setValue("viewStates", { + "retry-view": { + claimToken: "host-token", + mode: "architect", + currentApiConfigName: "cfg-a", + updatedAt: 1, + }, + }) + const getCustomModesSpy = vi + .spyOn(provider["customModesManager"], "getCustomModes") + .mockRejectedValue(new Error("custom modes read failed")) + + await provider["setViewStateId"]("retry-view", "host-token") + + // The id stays adopted: only the load aborted, and the entry is already stored under + // the stable name, so rolling back here would strand it under the temporary id. + expect(provider["viewStateId"]).toBe("retry-view") + expect(provider["viewLocalState"]).toStrictEqual({}) + expect(provider["pendingViewStateRestorations"].has("retry-view")).toBe(true) + + getCustomModesSpy.mockResolvedValue([]) + + // Same id, restoration still pending: the registration runs again, claim check included. + await provider["setViewStateId"]("retry-view", "host-token") + + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider["viewLocalState"].currentApiConfigName).toBe("cfg-a") + expect(provider["pendingViewStateRestorations"].has("retry-view")).toBe(false) + + getCustomModesSpy.mockRestore() + await provider.dispose() + }) + + it("keeps the same-id guard closed once a restoration completed", async () => { + const provider = createProvider() + await provider.contextProxy.setValue("viewStates", { + "loaded-view": { claimToken: "host-token", mode: "architect", updatedAt: 1 }, + }) + + await provider["setViewStateId"]("loaded-view", "host-token") + + expect(provider["viewLocalState"].mode).toBe("architect") + + // Change what is persisted behind this view's back; a completed restoration is not + // re-run by a repeated registration, so the buffer keeps the loaded selection. + await provider.contextProxy.setValue("viewStates", { + "loaded-view": { claimToken: "host-token", mode: "code", updatedAt: 2 }, + }) + await provider["setViewStateId"]("loaded-view", "host-token") + + expect(provider["viewLocalState"].mode).toBe("architect") + expect(provider["pendingViewStateRestorations"].size).toBe(0) + + await provider.dispose() + }) + it("applies the restored view fields to both the consumer state and the posted webview state", async () => { + const provider = createProvider() + // The shared values the restored view has to override on both read paths. + await provider.contextProxy.setValues({ mode: "code", currentApiConfigName: "shared-cfg" }) + await provider.contextProxy.setValue("viewStates", { + "overlay-view": { + claimToken: "host-token", + mode: "architect", + currentApiConfigName: "profile-a", + updatedAt: 1, + }, + }) + const profile: ProviderSettingsEntry = { + name: "profile-a", + id: "profile-a-id", + apiProvider: providerIdentifiers.anthropic, + } + const getProfileSpy = vi.spyOn(provider["providerSettingsManager"], "getProfile").mockResolvedValue(profile) + + await provider["setViewStateId"]("overlay-view", "host-token") + + expect(provider["viewLocalState"].mode).toBe("architect") + // The restoration is only real once the consumers see it: getState is the extension + // host's read path and getStateToPostToWebview is what the renderer paints, and the + // same three fields are read from the same overlay on both. + const state = await provider.getState() + expect(state.mode).toBe("architect") + expect(state.currentApiConfigName).toBe("profile-a") + expect(state.apiConfiguration).toMatchObject({ apiProvider: providerIdentifiers.anthropic }) + // The profile's own name is settings metadata, not part of its apiConfiguration. + expect(state.apiConfiguration).not.toHaveProperty("name") + + const posted = await provider.getStateToPostToWebview() + expect(posted.mode).toBe("architect") + expect(posted.currentApiConfigName).toBe("profile-a") + expect(posted.apiConfiguration).toMatchObject({ apiProvider: providerIdentifiers.anthropic }) + + getProfileSpy.mockRestore() + await provider.dispose() + }) + + it("keeps the shared values when a view holds no restored or buffered field", async () => { + const provider = createProvider() + await provider.contextProxy.setValues({ mode: "code", currentApiConfigName: "shared-cfg" }) + + // With an empty buffer the overlay is inert: the shared settings still answer. + const state = await provider.getState() + expect(state.mode).toBe("code") + expect(state.currentApiConfigName).toBe("shared-cfg") + + const posted = await provider.getStateToPostToWebview() + expect(posted.mode).toBe("code") + expect(posted.currentApiConfigName).toBe("shared-cfg") + + 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(true) + // 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["pendingViewStateWrites"] = [] + ClineProvider["viewStateWritesPump"] = 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["pendingViewStateWrites"] = [] + ClineProvider["viewStateWritesPump"] = 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["pendingViewStateWrites"] = [] + ClineProvider["viewStateWritesPump"] = 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["pendingViewStateWrites"] = [] + ClineProvider["viewStateWritesPump"] = 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(true) + + 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(true) + + // 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 () => { + // Claims go to the view's webview: attach the shared double so this exercises delivery + // rather than the no-view path. The post itself is spied below. + provider["view"] = mockWebviewView + // The webview has launched and attached its listener: claims are posted live from here on. + provider.isViewLaunched = true + + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + 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("resolves the persistence write only after the minted claim has been posted", async () => { + const gated = createProvider() + // The claim is posted to this view's webview: attach the shared double so the write resolves + // after a real delivery attempt. The post itself is spied below. + gated["view"] = mockWebviewView + // A launched webview: the claim post this write performs can actually be received. + gated.isViewLaunched = true + + let posted = false + const postSpy = vi.spyOn(gated, "postMessageToWebview").mockImplementation(async () => { + // A real webview post settles on a later turn: a write that reported success before + // this resolved would leave the view without the claim it just made durable. + await new Promise((resolve) => setTimeout(resolve, 0)) + posted = true + return true + }) + + await gated.saveViewState("mode", "architect") + + expect(storedEntries()[gated.viewId]?.claimToken).toEqual(expect.any(String)) + expect(posted).toBe(true) + + postSpy.mockRestore() + await gated.dispose() + }) + + test("logs a claim that cannot be delivered because no webview is attached", async () => { + const detached = createProvider() + // Nothing is attached in this harness: the post is dropped and the claim is durable + // anyway, so the gap has to reach the log instead of leaving a silent mismatch. + expect(detached["view"]).toBeUndefined() + const loggedBefore = (mockOutputChannel.appendLine as ReturnType).mock.calls.length + + await detached.saveViewState("mode", "architect") + + const loggedLines: string[] = (mockOutputChannel.appendLine as ReturnType).mock.calls + .slice(loggedBefore) + .map((call: unknown[]) => String(call[0])) + const claimLines = loggedLines.filter((line) => line.includes("[viewStateClaim]")) + expect(claimLines).toHaveLength(1) + expect(claimLines[0]).toContain("not delivered") + + await detached.dispose() + }) + + test("reports a claim as undelivered when the attached webview rejects the message", async () => { + // A hidden or closing webview resolves postMessage as false: the claim must be reported as + // not delivered and kept for retry, because the durable entry already carries this token. + const rejecting = createProvider() + const hiddenView = { + ...mockWebviewView, + webview: { ...mockWebviewView.webview, postMessage: vi.fn().mockResolvedValue(false) }, + } + rejecting["view"] = hiddenView + // Launched but closing: the post is attempted and the webview refuses it. + rejecting.isViewLaunched = true + const loggedBefore = (mockOutputChannel.appendLine as ReturnType).mock.calls.length + + const delivered = await rejecting["postViewStateClaimToWebview"]("hidden-view", "host-minted-token") + + expect(delivered).toBe("queued") + expect(hiddenView.webview.postMessage).toHaveBeenCalledWith({ + type: "viewStateClaim", + viewStateId: "hidden-view", + viewStateClaim: "host-minted-token", + }) + const loggedLines: string[] = (mockOutputChannel.appendLine as ReturnType).mock.calls + .slice(loggedBefore) + .map((call: unknown[]) => String(call[0])) + expect(loggedLines.some((line) => line.includes("[viewStateClaim] Claim for hidden-view"))).toBe(true) + expect(loggedLines.some((line) => line.includes("the webview rejected the message"))).toBe(true) + expect(rejecting["pendingViewStateClaims"].get("hidden-view")).toBe("host-minted-token") + + await rejecting.dispose() + }) + + test("redelivers the queued claim with the same token when the webview launches", async () => { + // Nothing is attached yet: the claim is durable but undelivered, so it is queued. Once a + // webview attaches it must receive the exact token the entry stores, not a re-minted one. + const detached = createProvider() + expect(detached["view"]).toBeUndefined() + + await detached.saveViewState("mode", "architect") + + const storedClaim = storedEntries()[detached.viewId]?.claimToken + expect(storedClaim).toEqual(expect.any(String)) + expect(detached["pendingViewStateClaims"].get(detached.viewId)).toBe(storedClaim) + + const attachedView = { + ...mockWebviewView, + webview: { ...mockWebviewView.webview, postMessage: vi.fn().mockResolvedValue(true) }, + } + await detached.resolveWebviewView(attachedView) + + // Attaching the view is not the delivery point: the renderer only has a message listener + // once it launches, so the held claim goes out from the webviewDidLaunch handler. + const launchedMessageHandler = (attachedView.webview.onDidReceiveMessage as ReturnType).mock + .calls[0][0] + await launchedMessageHandler({ type: "webviewDidLaunch" }) + + const claimPosts = attachedView.webview.postMessage.mock.calls + .map((call: unknown[]) => call[0]) + .filter( + (message: unknown): message is { type: string; viewStateId: string; viewStateClaim: string } => + typeof message === "object" && + message !== null && + (message as { type?: unknown }).type === "viewStateClaim", + ) + expect(claimPosts).toHaveLength(1) + expect(claimPosts[0]).toEqual({ + type: "viewStateClaim", + viewStateId: detached.viewId, + viewStateClaim: storedClaim, + }) + expect(detached["pendingViewStateClaims"].size).toBe(0) + + await detached.dispose() + }) + + test("holds a claim minted after a renderer replacement and delivers it on the next launch", async () => { + // The interleaving the queue exists for: replacing the view drops the renderer's message + // listener, a durable view-state mutation mints a claim while the replacement renderer has + // no listener yet, and a post that resolves true against the page being replaced deletes a + // claim that renderer never received, so its launch cannot adopt this view's state. + const replaced = createProvider() + const firstView = { + ...mockWebviewView, + webview: { + ...mockWebviewView.webview, + onDidReceiveMessage: vi.fn(), + postMessage: vi.fn().mockResolvedValue(true), + }, + } + await replaced.resolveWebviewView(firstView) + const firstLaunch = (firstView.webview.onDidReceiveMessage as ReturnType).mock.calls[0][0] + await firstLaunch({ type: "webviewDidLaunch" }) + expect(replaced.isViewLaunched).toBe(true) + + // A different view replaces the first one. The replacement renderer has not sent + // `webviewDidLaunch`, so nothing listens yet, yet its postMessage still resolves true - + // exactly the webview that swallows a claim the host believes was delivered. + const secondView = { + ...mockWebviewView, + webview: { + ...mockWebviewView.webview, + onDidReceiveMessage: vi.fn(), + postMessage: vi.fn().mockResolvedValue(true), + }, + } + await replaced.resolveWebviewView(secondView) + expect(replaced.isViewLaunched).toBe(false) + + const result = await replaced.saveViewState("mode", "architect") + const mintedClaim = storedEntries()[replaced.viewId]?.claimToken + + // Held rather than posted: the entry keeps the token it just minted and the queue holds + // that same token for the launch that can actually receive it. + expect(result).toEqual({ persisted: true, claim: "queued" }) + expect(mintedClaim).toEqual(expect.any(String)) + expect(replaced["pendingViewStateClaims"].get(replaced.viewId)).toBe(mintedClaim) + expect( + secondView.webview.postMessage.mock.calls + .map((call: unknown[]) => call[0]) + .filter( + (message: unknown): message is { type: string; viewStateId: string; viewStateClaim: string } => + typeof message === "object" && + message !== null && + (message as { type?: unknown }).type === "viewStateClaim", + ), + ).toEqual([]) + + // The replacement renderer launches: the claim it never received arrives carrying the token + // the record already stores, which is what lets that launch adopt this view's state. + const secondLaunch = (secondView.webview.onDidReceiveMessage as ReturnType).mock.calls[0][0] + await secondLaunch({ type: "webviewDidLaunch" }) + + const claimPosts = secondView.webview.postMessage.mock.calls + .map((call: unknown[]) => call[0]) + .filter( + (message: unknown): message is { type: string; viewStateId: string; viewStateClaim: string } => + typeof message === "object" && + message !== null && + (message as { type?: unknown }).type === "viewStateClaim", + ) + expect(claimPosts).toEqual([ + { type: "viewStateClaim", viewStateId: replaced.viewId, viewStateClaim: mintedClaim }, + ]) + expect(replaced["pendingViewStateClaims"].size).toBe(0) + + await replaced.dispose() + }) + + test("lowers the launched flag when recovery replaces the renderer's HTML", async () => { + // Assigning new HTML discards the live document, listener included. A provider that still + // believes the renderer is launched posts claims into a page that is being replaced. + // Recovery schedules its own confirmation timer; fake timers keep it off the run so it + // cannot log after the environment is gone. + vi.useFakeTimers() + const recovered = createProvider() + recovered["view"] = mockWebviewView + recovered.isViewLaunched = true + recovered["getWebviewHtml"] = vi.fn().mockResolvedValue("recovered-html") + const postMessageSpy = vi.spyOn(recovered, "postMessageToWebview").mockResolvedValue(true) + + await recovered["reloadWebviewForRecovery"]() + + expect(recovered.isViewLaunched).toBe(false) + + // A claim minted after the swap is held for the regenerated renderer's own launch instead + // of being posted into a page whose listener is already gone. + const result = await recovered.saveViewState("mode", "architect") + const storedClaim = storedEntries()[recovered.viewId]?.claimToken + expect(result).toEqual({ persisted: true, claim: "queued" }) + expect(storedClaim).toEqual(expect.any(String)) + expect(recovered["pendingViewStateClaims"].get(recovered.viewId)).toBe(storedClaim) + expect( + postMessageSpy.mock.calls + .map((call: unknown[]) => call[0]) + .filter( + (message: unknown): message is { type: string; viewStateId: string; viewStateClaim: string } => + typeof message === "object" && + message !== null && + (message as { type?: unknown }).type === "viewStateClaim", + ), + ).toEqual([]) + + // The regenerated renderer reports its own launch and the held claim reaches it. + recovered.isViewLaunched = true + await recovered.deliverPendingViewStateClaims() + expect( + postMessageSpy.mock.calls + .map((call: unknown[]) => call[0]) + .filter( + (message: unknown): message is { type: string; viewStateId: string; viewStateClaim: string } => + typeof message === "object" && + message !== null && + (message as { type?: unknown }).type === "viewStateClaim", + ), + ).toEqual([{ type: "viewStateClaim", viewStateId: recovered.viewId, viewStateClaim: storedClaim }]) + + recovered["clearWebviewRecoveryConfirmation"]() + postMessageSpy.mockRestore() + await recovered.dispose() + vi.useRealTimers() + }) + test("reports a queued claim on the save result when the webview has not launched", async () => { + // The persistence-integrity gap this result exists to close: the durable entry carries a + // claim the webview has never been handed. A save that resolves as a complete success + // hides an identity the next launch still has to deliver. + const detached = createProvider() + detached["view"] = mockWebviewView + const postMessageSpy = vi.spyOn(detached, "postMessageToWebview").mockResolvedValue(true) + + const result = await detached.saveViewState("mode", "architect") + + expect(result).toEqual({ persisted: true, claim: "queued" }) + expect(storedEntries()[detached.viewId]?.claimToken).toEqual(expect.any(String)) + expect(detached["pendingViewStateClaims"].get(detached.viewId)).toBe( + storedEntries()[detached.viewId]?.claimToken, + ) + // Nothing was posted: the renderer has no listener yet, and a post that resolved true + // here would delete the claim the launch flush exists to deliver. + expect(postMessageSpy).not.toHaveBeenCalled() + + postMessageSpy.mockRestore() + await detached.dispose() + }) + + test("reports a delivered claim on the save result once the webview has launched", async () => { + const launched = createProvider() + launched["view"] = mockWebviewView + launched.isViewLaunched = true + const postMessageSpy = vi.spyOn(launched, "postMessageToWebview").mockResolvedValue(true) + + const result = await launched.saveViewState("mode", "architect") + + expect(result).toEqual({ persisted: true, claim: "delivered" }) + expect(launched["pendingViewStateClaims"].size).toBe(0) + + postMessageSpy.mockRestore() + await launched.dispose() + }) + + test("reports a queued claim on the registration result of an unlaunched view", async () => { + // Re-keying a pre-launch selection onto its stable id re-affirms that entry's claim. Until + // the renderer launches the reaffirmation is held, and the registration result has to say + // that rather than report an adoption whose identity never reached the webview. + const minted = createProvider() + minted["view"] = mockWebviewView + const postMessageSpy = vi.spyOn(minted, "postMessageToWebview").mockResolvedValue(true) + await minted.saveViewState("mode", "architect") + const mintedClaim = storedEntries()[minted.viewId]?.claimToken + + const registered = await minted["setViewStateId"]("rekeyed-view") + + expect(registered).toEqual({ adopted: true, claim: "queued" }) + expect(storedEntries()["rekeyed-view"]?.claimToken).toBe(mintedClaim) + expect(postMessageSpy).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "viewStateClaim", viewStateId: "rekeyed-view" }), + ) + + postMessageSpy.mockRestore() + await minted.dispose() + }) + + test("reports a delivered claim on the registration result of a launched view", async () => { + const launched = createProvider() + launched["view"] = mockWebviewView + launched.isViewLaunched = true + const postMessageSpy = vi.spyOn(launched, "postMessageToWebview").mockResolvedValue(true) + await launched.saveViewState("mode", "architect") + const mintedClaim = storedEntries()[launched.viewId]?.claimToken + postMessageSpy.mockClear() + + const registered = await launched["setViewStateId"]("launched-view") + + expect(registered).toEqual({ adopted: true, claim: "delivered" }) + expect(storedEntries()["launched-view"]?.claimToken).toBe(mintedClaim) + expect( + postMessageSpy.mock.calls + .map(([message]) => message) + .filter((message) => message.type === "viewStateClaim"), + ).toEqual([{ type: "viewStateClaim", viewStateId: "launched-view", viewStateClaim: mintedClaim }]) + + postMessageSpy.mockRestore() + await launched.dispose() + }) + + 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 }, + }) + + // Claims go to the view's webview: attach the shared double so this exercises delivery + // rather than the no-view path. The post itself is spied below. + provider["view"] = mockWebviewView + // The re-bind hands the fresh claim to a launched webview. + provider.isViewLaunched = true + + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) + + 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() + } + }) + }) + + const seedPreLaunchEntry = async (provider: ClineProvider) => { + // A pre-launch selection under the instance's temporary id, authored by this instance. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(true) + return provider.viewId + } + + // Moves the provider's view-state generation while its own storage write is in flight, + // which is what a dispose or a re-registration during the write does. The double delegates + // to the real setter so the write still lands: replacing it outright would hide the move. + const moveGenerationDuringViewStatesWrite = (provider: ClineProvider) => { + const original = provider.contextProxy.setValue.bind(provider.contextProxy) + vi.spyOn(provider.contextProxy, "setValue").mockImplementation(async (...args: unknown[]) => { + await (original as (...a: unknown[]) => Promise)(...args) + if (args[0] === "viewStates") { + provider["viewStateGeneration"]++ + } + }) + } + it("keeps a pre-launch selection when registration re-keys it onto an existing stable entry", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + const seeded = provider.contextProxy.getValue("viewStates") ?? {} + await provider.contextProxy.setValue("viewStates", { + ...seeded, + "stable-view": { claimToken: "host-token", updatedAt: 2 }, + }) + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + expect(mockContext.globalState.get("viewStates")).toMatchObject({ + "stable-view": { mode: "architect", claimToken: "host-token" }, + }) + + await provider.dispose() + }) + + it("puts a re-key back under the temporary id when the view died before the entry reached a renderer", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const temporaryId = await seedPreLaunchEntry(provider) + moveGenerationDuringViewStatesWrite(provider) + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + // The move committed before the generation check ran. Leaving it applied binds the entry + // to a view id no renderer will ever present, and clears nothing: a later launch reads + // the temporary id as unauthored and the selection is stranded under the dead claim. + const states = mockContext.globalState.get>("viewStates") ?? {} + expect(states[temporaryId]?.mode).toBe("architect") + expect(states["stable-view"]).toBeUndefined() + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(true) + + await provider.dispose() + }) + + it("clears authorship when the dead view had already merged the pre-launch entry", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const temporaryId = await seedPreLaunchEntry(provider) + const seeded = provider.contextProxy.getValue("viewStates") ?? {} + await provider.contextProxy.setValue("viewStates", { + ...seeded, + "stable-view": { claimToken: "host-token", updatedAt: 2 }, + }) + moveGenerationDuringViewStatesWrite(provider) + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + // A merged entry cannot be pulled back out whole, so the temporary name is gone either + // way; authorship must not survive it, or every later launch keeps treating this view as + // the author of an entry that no longer exists under the temporary id. + const states = + mockContext.globalState.get>("viewStates") ?? {} + expect(states["stable-view"]).toMatchObject({ mode: "architect", claimToken: "host-token" }) + expect(states[temporaryId]).toBeUndefined() + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(false) + + await provider.dispose() + }) + + it("leaves the entry under the temporary id and keeps authorship when the compensating re-key write also fails", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const temporaryId = await seedPreLaunchEntry(provider) + moveGenerationDuringViewStatesWrite(provider) + + // The move is already stored when the generation check runs, so the recovery path is the + // compensating write. Reject the storage update whose payload puts the entry back under the + // temporary id and leave every other write behaving normally - payload-keyed so it hits that + // write and not the queue calls that precede it. + let compensatingWrites = 0 + const mementoUpdate = mockContext.globalState.update + mockContext.globalState.update = ((key: string, value: unknown) => { + const payload = value as Record | undefined + if (key === "viewStates" && payload && Object.prototype.hasOwnProperty.call(payload, temporaryId)) { + compensatingWrites++ + return Promise.reject(new Error("storage rejected the compensating write")) + } + return mementoUpdate(key, value) + }) as typeof mockContext.globalState.update + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + expect(compensatingWrites).toBe(1) // the recovery write was attempted exactly once + const stored = mockContext.globalState.get>("viewStates") ?? {} + const cached = + (provider.contextProxy.getValue("viewStates") as Record | undefined) ?? {} + // The recovery write never landed, so storage still holds the entry under the stable id from the + // forward write. The rollback exists so the cache ContextProxy keeps for the key says the same + // thing: without it the cache claims the entry moved back under a temporary id that durable + // storage never received, and every later read of the key serves a value nothing backs. + expect(stored["stable-view"]?.mode).toBe("architect") + expect(cached["stable-view"]?.mode).toBe("architect") + expect(cached[temporaryId]).toBeUndefined() + // Authorship stays claimed: storage still holds the entry under the stable id and a later launch + // has to re-key it, which is what the forward path already guarantees for its own failures. + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(true) + + await provider.dispose() + }) + + it("keeps the re-key applied and clears authorship while the view generation is live", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const temporaryId = await seedPreLaunchEntry(provider) + + await provider["rekeyPersistedViewStateEntry"]("stable-view") + + const states = mockContext.globalState.get>("viewStates") ?? {} + expect(states["stable-view"]?.mode).toBe("architect") + expect(states[temporaryId]).toBeUndefined() + expect(provider["wroteUnderTemporaryViewStateId"]).toBe(false) + + await provider.dispose() + }) + + describe("view state writes that outlive the provider", () => { + const createProvider = () => + new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const storedEntries = () => + mockContext.globalState.get>("viewStates") ?? {} + + beforeEach(() => { + // The write queue is static and shared: start from a drained queue so a closure an + // earlier test left on it cannot hold this one open. + ClineProvider["pendingViewStateWrites"] = [] + ClineProvider["viewStateWritesPump"] = Promise.resolve() + }) + + it("drops a queued write instead of applying it to a disposed provider", async () => { + const blocked = createProvider() + let releaseBlocked!: () => void + const blockedGate = new Promise((resolve) => { + releaseBlocked = resolve + }) + // The first provider's storage write stalls, so the shared queue stays taken and + // the second provider's closure waits behind it. + const blockedWriteSpy = vi.spyOn(blocked.contextProxy, "setValue").mockImplementation(async () => { + await blockedGate + }) + + const victim = createProvider() + const victimWriteSpy = vi.spyOn(victim.contextProxy, "setValue") + const postSpy = vi.spyOn(victim, "postMessageToWebview").mockResolvedValue(true) + + const blockedWrite = blocked.saveViewState("mode", "plan") + // Queued behind the stalled write: none of the victim's work has started yet. + const victimWrite = victim.saveViewState("mode", "architect") + + await victim.dispose() + releaseBlocked() + await victimWrite + await blockedWrite + + // The queue only advances this far once the view is gone: no global-state write, + // no provider-field mutation, no claim handed to a webview that no longer exists, + // and nothing stored under the view's id. + expect(victimWriteSpy).not.toHaveBeenCalled() + expect(victim["wroteUnderTemporaryViewStateId"]).toBe(false) + expect(storedEntries()[victim.viewId]).toBeUndefined() + expect( + postSpy.mock.calls.map(([message]) => message).filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + + blockedWriteSpy.mockRestore() + victimWriteSpy.mockRestore() + postSpy.mockRestore() + await blocked.dispose() + }) + + it("leaves a queued re-key alone when the provider is disposed while it waits", async () => { + const blocked = createProvider() + let releaseBlocked!: () => void + const blockedGate = new Promise((resolve) => { + releaseBlocked = resolve + }) + const blockedWriteSpy = vi.spyOn(blocked.contextProxy, "setValue").mockImplementation(async () => { + await blockedGate + }) + + const victim = createProvider() + // Author a pre-launch entry for real, so the re-key has work to do. + await victim["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + expect(storedEntries()[victim.viewId]?.mode).toBe("architect") + expect(victim["wroteUnderTemporaryViewStateId"]).toBe(true) + + const blockedWrite = blocked.saveViewState("mode", "plan") + // The re-key is queued behind the stalled write and has not started yet. + const victimRekey = victim["rekeyPersistedViewStateEntry"]("stable-view") + + await victim.dispose() + releaseBlocked() + await victimRekey + await blockedWrite + + // Neither the move nor the authorship clearing may run after dispose: the entry + // stays under the id it was written under and the flag keeps its value. + expect(storedEntries()[victim.viewId]?.mode).toBe("architect") + expect(storedEntries()["stable-view"]).toBeUndefined() + expect(victim["wroteUnderTemporaryViewStateId"]).toBe(true) + + blockedWriteSpy.mockRestore() + await blocked.dispose() + }) + + it("skips post-write field mutation and claim delivery for a disposed provider", async () => { + const victim = createProvider() + let releaseWrite!: () => void + const writeGate = new Promise((resolve) => { + releaseWrite = resolve + }) + // Only the viewStates write is held open, so the rest of dispose() still runs. + // The write must actually land for the rollback below to be observable, so the gate + // forwards to the real setter instead of swallowing the call. + const passThrough = victim.contextProxy.setValue.bind(victim.contextProxy) + const setValueSpy = vi.spyOn(victim.contextProxy, "setValue").mockImplementation(async (key, value) => { + if (key === "viewStates") { + await writeGate + } + + return passThrough(key, value) + }) + const postSpy = vi.spyOn(victim, "postMessageToWebview").mockResolvedValue(true) + + const save = victim.saveViewState("mode", "architect") + // Let the queued closure reach its storage write, then dispose while it is in + // flight: the durable write is already issued, the effects after it are not. + await vi.waitFor(() => { + expect(setValueSpy.mock.calls.some(([key]) => key === "viewStates")).toBe(true) + }) + + await victim.dispose() + releaseWrite() + const saveResult = await save + + expect(setValueSpy.mock.calls.some(([key]) => key === "viewStates")).toBe(true) + // The write itself was allowed to land; nothing applied after it. + expect(victim["wroteUnderTemporaryViewStateId"]).toBe(false) + expect( + postSpy.mock.calls.map(([message]) => message).filter((message) => message.type === "viewStateClaim"), + ).toHaveLength(0) + + // The write landed and was then undone. This provider minted the entry's claim, no + // renderer was ever handed it, and the only provider that could post it is gone, so the + // record goes back to what it held before the write instead of carrying a binding nobody + // can ever present. The restore is the second viewStates write. + expect(setValueSpy.mock.calls.filter(([key]) => key === "viewStates")).toHaveLength(2) + expect(storedEntries()).not.toHaveProperty(victim["viewStateId"]) + expect(saveResult).toEqual({ persisted: false, claim: "none" }) + + setValueSpy.mockRestore() + postSpy.mockRestore() + }) + + it("leaves the key adoptable by the next provider after rolling an orphaned write back", async () => { + const victim = createProvider() + const strandedKey = victim["viewStateId"] + let releaseWrite!: () => void + const writeGate = new Promise((resolve) => { + releaseWrite = resolve + }) + const passThrough = victim.contextProxy.setValue.bind(victim.contextProxy) + const setValueSpy = vi.spyOn(victim.contextProxy, "setValue").mockImplementation(async (key, value) => { + if (key === "viewStates") { + await writeGate + } + + return passThrough(key, value) + }) + + const save = victim.saveViewState("mode", "architect") + await vi.waitFor(() => { + expect(setValueSpy.mock.calls.some(([key]) => key === "viewStates")).toBe(true) + }) + + await victim.dispose() + releaseWrite() + + expect(await save).toEqual({ persisted: false, claim: "none" }) + setValueSpy.mockRestore() + + // Before the rollback the next provider could not take this key at all: the record still + // carried the dead provider's token, and a registration whose presented claim does not + // match is refused side-effect free, so the entry was durable state no provider could + // restore. With the write undone the key is simply absent. + expect(storedEntries()).not.toHaveProperty(strandedKey) + + const fresh = createProvider() + const registration = await fresh.setViewStateId(strandedKey) + expect(registration.adopted).toBe(true) + expect(storedEntries()[strandedKey]).toBeUndefined() + + // And the key now takes a binding this provider can actually hand to its own renderer. + const saved = await fresh.saveViewState("mode", "code") + expect(saved).toEqual({ persisted: true, claim: "queued" }) + expect(storedEntries()[strandedKey]?.mode).toBe("code") + expect(storedEntries()[strandedKey]?.claimToken).toEqual(expect.any(String)) + + await fresh.dispose() + }) + + it("keeps a durable write a concurrent provider already re-bound and reports the dropped claim", async () => { + const victim = createProvider() + const writeKey = victim["viewStateId"] + let releaseWrite!: () => void + const writeGate = new Promise((resolve) => { + releaseWrite = resolve + }) + // The storage write lands carrying another provider's binding, as if a concurrent + // registration had re-bound this key while the write was in flight. + const passThrough = victim.contextProxy.setValue.bind(victim.contextProxy) + const setValueSpy = vi.spyOn(victim.contextProxy, "setValue").mockImplementation(async (key, value) => { + if (key === "viewStates") { + await writeGate + return passThrough(key, { [writeKey]: { claimToken: "winner", mode: "plan", updatedAt: 1 } }) + } + + return passThrough(key, value) + }) + + const save = victim.saveViewState("mode", "architect") + await vi.waitFor(() => { + expect(setValueSpy.mock.calls.some(([key]) => key === "viewStates")).toBe(true) + }) + await victim.dispose() + releaseWrite() + + expect(await save).toEqual({ persisted: true, claim: "dropped" }) + // Only the original write ran: the rollback is skipped rather than deleting a binding + // that now belongs to a provider that is still alive. + expect(setValueSpy.mock.calls.filter(([key]) => key === "viewStates")).toHaveLength(1) + expect(storedEntries()[writeKey]).toMatchObject({ claimToken: "winner", mode: "plan" }) + + setValueSpy.mockRestore() + }) + + it("detaches a provider's queued writes when it disposes", async () => { + const blocked = createProvider() + let releaseBlocked!: () => void + const blockedGate = new Promise((resolve) => { + releaseBlocked = resolve + }) + const blockedWriteSpy = vi.spyOn(blocked.contextProxy, "setValue").mockImplementation(async () => { + await blockedGate + }) + + const victim = createProvider() + const victimWriteSpy = vi.spyOn(victim.contextProxy, "setValue") + + const blockedWrite = blocked.saveViewState("mode", "plan") + const victimWrite = victim.saveViewState("mode", "architect") + + // The victim's write is queued behind the stalled one and has not run yet: its entry is on + // the shared queue and its storage call has not started. + await vi.waitFor(() => { + expect(victim["scheduledViewStateWrites"].length).toBeGreaterThan(0) + }) + expect(victimWriteSpy).not.toHaveBeenCalled() + + // The entry the victim queued points at it, and it is still on the shared array because + // the stalled write ahead of it has not released the queue. + const queued = victim["scheduledViewStateWrites"][0] + expect(queued.provider).toBe(victim) + expect(ClineProvider["pendingViewStateWrites"]).toContain(queued) + + await victim.dispose() + + // Detachment is what keeps a disposed provider off the queue: the entry leaves the shared + // array and its provider slot is cleared, so nothing still queued can reach the provider. + expect(ClineProvider["pendingViewStateWrites"]).not.toContain(queued) + expect(queued.provider).toBeNull() + expect(victim["scheduledViewStateWrites"]).toHaveLength(0) + + releaseBlocked() + await victimWrite + await blockedWrite + + expect(victimWriteSpy).not.toHaveBeenCalled() + expect(storedEntries()[victim["viewId"]]).toBeUndefined() + + blockedWriteSpy.mockRestore() + victimWriteSpy.mockRestore() + await blocked.dispose() + }) + it("detaches the provider's entry after a queued write resolves", async () => { + // A long-lived view must not accumulate the history of every write it scheduled, + // so the queue step drops the entry it has finished running. + const keeper = createProvider() + + await keeper.saveViewState("mode", "code") + + expect(keeper["scheduledViewStateWrites"]).toHaveLength(0) + expect(ClineProvider["pendingViewStateWrites"].some((entry) => entry.provider === keeper)).toBe(false) + + await keeper.dispose() + }) + + it("detaches the owner's entry when a queued write rejects", async () => { + const blocked = createProvider() + let releaseBlocked!: () => void + const blockedGate = new Promise((resolve) => { + releaseBlocked = resolve + }) + const blockedWriteSpy = vi.spyOn(blocked.contextProxy, "setValue").mockImplementation(async () => { + await blockedGate + }) + + const owner = createProvider() + // The owner's write runs behind the stalled one and then fails at storage. + const ownerWriteSpy = vi + .spyOn(owner.contextProxy, "setValue") + .mockRejectedValue(new Error("storage write failed")) + + const blockedWrite = blocked.saveViewState("mode", "plan") + const ownerWrite = owner.saveViewState("mode", "architect") + + await vi.waitFor(() => { + expect(owner["scheduledViewStateWrites"].length).toBeGreaterThan(0) + }) + const entry = owner["scheduledViewStateWrites"][0] + + releaseBlocked() + // The failure still surfaces to the caller that scheduled the write, and the queue + // keeps advancing so a later view's write still runs. + await expect(ownerWrite).rejects.toThrow("storage write failed") + await blockedWrite + + // Detachment happens whether the write resolved or rejected. Without it a view whose + // write rejected kept the entry, its run closure, and every value that closure + // captured for as long as the process lived. + expect(owner["scheduledViewStateWrites"]).toHaveLength(0) + expect(ClineProvider["pendingViewStateWrites"]).not.toContain(entry) + + blockedWriteSpy.mockRestore() + ownerWriteSpy.mockRestore() + await owner.dispose() + await blocked.dispose() + }) + }) + describe("webview-originated configuration boundary", () => { + it("refuses a viewStates key carried by a newTask taskConfiguration", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + // The host record the posted configuration must not be able to rewrite: a view's + // selections and the claim token that binds them belong to the view that minted them. + await provider.contextProxy.setValue("viewStates", { + "host-view": { claimToken: "host-token", mode: "code", updatedAt: 5 }, + }) + + const view = { + ...mockWebviewView, + webview: { + ...mockWebviewView.webview, + onDidReceiveMessage: vi.fn(), + postMessage: vi.fn().mockResolvedValue(true), + }, + } + await provider.resolveWebviewView(view) + const messageHandler = (view.webview.onDidReceiveMessage as ReturnType).mock.calls[0][0] + + await messageHandler({ + type: "newTask", + text: "hello", + taskConfiguration: { + viewStates: { "host-view": { claimToken: "stolen", mode: "plan", updatedAt: 9 } }, + mode: "program", + }, + }) + + const stored = + mockContext.globalState.get>("viewStates") ?? {} + // The webview could not smuggle the host-only key in through the task configuration: + // the record still carries the host's own token and selections. + expect(stored["host-view"]).toMatchObject({ claimToken: "host-token", mode: "code" }) + // The legitimate part of the same payload was applied. + expect(await provider.getMode()).toBe("program") + + await provider.dispose() + }) + }) }) describe("webviewMessageHandler no-floating-promises coverage", () => { @@ -8043,7 +9664,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { } as any) const upsertSpy = vi.spyOn(provider, "upsertProviderProfile").mockResolvedValue("profile-id") vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) ;(provider as any).providerSettingsManager = { listConfig: vi.fn().mockResolvedValue([]), } @@ -8147,7 +9768,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { const { getCachedZooCodeToken } = await import("../../../services/zoo-code-auth") vi.mocked(getCachedZooCodeToken).mockReturnValue("current-token") const handleSpy = vi.spyOn(provider, "handleZooCodeCallback").mockResolvedValue(undefined) - const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(true) ;(provider as any).providerSettingsManager = { listConfig: vi diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index bf50f08786..b7a9ee4373 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -128,6 +128,8 @@ const mockClineProvider = { }, log: vi.fn(), postStateToWebview: vi.fn(), + // The webviewDidLaunch handler flushes claims the provider queued before the renderer launched. + deliverPendingViewStateClaims: vi.fn().mockResolvedValue(undefined), resolveWebviewThemeFixtureProbe: vi.fn(), getCurrentTask: vi.fn(), getTaskWithId: vi.fn(), @@ -2804,6 +2806,24 @@ describe("webviewMessageHandler - telemetrySetting", () => { expect(calls.at(-1)).toEqual([true]) }) + // Review follow-up on the attach-time claim retry: a claim minted before the renderer launched is + // held by the provider, because a message posted before the webview attaches its listener can be + // lost while postMessage still resolves true. The flush therefore belongs to webviewDidLaunch, and + // it has to run only after isViewLaunched is set - flushing earlier would post back into the window + // the gate exists to close. + it("flushes the provider's pending view-state claims on webviewDidLaunch, after isViewLaunched is set", async () => { + vi.mocked(mockClineProvider.customModesManager.getCustomModes).mockResolvedValue([]) + let launchedWhenFlushed: boolean | undefined + vi.mocked(mockClineProvider.deliverPendingViewStateClaims).mockImplementationOnce(async () => { + launchedWhenFlushed = mockClineProvider.isViewLaunched + }) + + await webviewMessageHandler(mockClineProvider, { type: "webviewDidLaunch" }) + + expect(mockClineProvider.deliverPendingViewStateClaims).toHaveBeenCalledTimes(1) + expect(launchedWhenFlushed).toBe(true) + }) + // CodeRabbit follow-up on the finding #12 fix: webviewDidLaunch's telemetry init read state // via an async provider.getStateToPostToWebview().then(...) continuation, outside // telemetrySettingQueue -- so it could resolve after a concurrent "telemetrySetting" message @@ -3055,8 +3075,35 @@ describe("webviewMessageHandler - renameTaskWithId", () => { }) }) -// The display settings are written like any other setting, but the webview is not trusted: a value that its own -// controls cannot produce (a hand-written message) is ignored before anything is done for it. +describe("webviewMessageHandler - non-portable settings keys", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("refuses viewStates while applying the portable keys of the same payload", async () => { + // The per-view selections and the claim token that binds them are minted by the host, and the + // settings-file path already refuses them. A posted updateSettings is the same untrusted input, + // so writing them straight into the shared store would let a webview choose which persisted view + // entry - and with which identity - it owns. + await webviewMessageHandler(mockClineProvider, { + type: "updateSettings", + updatedSettings: { + viewStates: { "some-view": { claimToken: "webview-minted", mode: "code", updatedAt: 1 } }, + customInstructions: "carry on", + }, + }) + + const writtenKeys = vi.mocked(mockClineProvider.contextProxy.setValue).mock.calls.map((call) => String(call[0])) + expect(writtenKeys).not.toContain("viewStates") + expect(writtenKeys).toContain("customInstructions") + expect(mockClineProvider.log).toHaveBeenCalledWith( + expect.stringContaining("Ignoring non-portable settings key viewStates"), + ) + }) +}) + +// The display settings are written like any other setting, but the webview is not trusted: a +// value its own controls cannot produce (a hand-written message) is ignored before storage. describe("updateSettings with the display settings", () => { const save = (updatedSettings: Record) => webviewMessageHandler(mockClineProvider, { type: "updateSettings", updatedSettings }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4adb29264b..be216a6472 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -41,6 +41,7 @@ import { isInvalidDisplayValue } from "../config/displaySettings" import { importRooTaskHistory } from "../task-persistence/importRooTaskHistory" import { ClineProvider } from "./ClineProvider" +import { isNonPortableSettingsKey } from "../config/ContextProxy" import { findOriginalContent } from "./stripOriginalContent" import { handleCheckpointRestoreOperation } from "./checkpointRestoreHandler" import { assertRecordedCheckpoint } from "../checkpoints/recordedCheckpoint" @@ -701,6 +702,17 @@ export const webviewMessageHandler = async ( ) provider.isViewLaunched = true + // Claims minted before this launch were held by the provider, because posting one earlier + // could be lost while postMessage still resolved true. The listener exists now, so this is + // the point where the queued posts can actually be received. A failed flush must not fail + // the launch, so it is contained here. + try { + await provider.deliverPendingViewStateClaims() + } catch (error) { + provider.log( + `[viewStateClaim] Flushing pending claims on launch failed: ${error instanceof Error ? error.message : String(error)}`, + ) + } break case "webviewHeartbeat": // Confirm renderer liveness and advance the watchdog recovery health window. @@ -774,6 +786,18 @@ export const webviewMessageHandler = async ( } for (const [key, value] of Object.entries(message.updatedSettings)) { + if (isNonPortableSettingsKey(key)) { + // Per-view selections and the claim token that binds them are minted and held by the host, + // and the settings-file path already refuses them. A posted updateSettings is the same + // untrusted input, so writing them here would let a webview choose which persisted view + // entry - and with which identity - it owns, bypassing the claim check in setViewStateId. + provider.log( + `[updateSettings] Ignoring non-portable settings key ${key} posted by the webview.`, + ) + + continue + } + // An invalid display value is ignored, before anything is done for it. if (isInvalidDisplayValue(key, value)) { provider.log(`[updateSettings] Ignored an invalid value for ${key}`) diff --git a/src/extension/__tests__/api-configuration.spec.ts b/src/extension/__tests__/api-configuration.spec.ts index a10bfe60ac..cdd496a49e 100644 --- a/src/extension/__tests__/api-configuration.spec.ts +++ b/src/extension/__tests__/api-configuration.spec.ts @@ -12,6 +12,19 @@ vi.mock("@roo-code/ipc", () => ({ })) describe("API - configuration", () => { + // API needs an OutputChannel it can log to. The shared helper in test-utils/provider.ts types one the + // same way, so this double carries every vscode.OutputChannel member and stays type-checked instead + // of cast (AGENTS.md: new code avoids any-style assertions). + const makeOutputChannel = (): vscode.OutputChannel => ({ + name: "test-output", + append: vi.fn(), + appendLine: vi.fn(), + replace: vi.fn(), + clear: vi.fn(), + show: vi.fn(), + hide: vi.fn(), + dispose: vi.fn(), + }) it("persists every supplied mode API config mapping", async () => { const setValues = vi.fn().mockResolvedValue(undefined) const saveConfig = vi.fn().mockResolvedValue("default-id") @@ -25,7 +38,7 @@ describe("API - configuration", () => { providerSettingsManager: { saveConfig, setModeConfig }, postStateToWebview, } as unknown as ClineProvider - const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel + const outputChannel = makeOutputChannel() const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.setConfiguration({ @@ -59,7 +72,7 @@ describe("API - configuration", () => { providerSettingsManager: { saveConfig, setModeConfig }, postStateToWebview, } as unknown as ClineProvider - const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel + const outputChannel = makeOutputChannel() const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.setConfiguration({ currentApiConfigName: "default" }) @@ -83,7 +96,7 @@ describe("API - configuration", () => { }, postStateToWebview: vi.fn().mockResolvedValue(undefined), } as unknown as ClineProvider - const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel + const outputChannel = makeOutputChannel() return { api: new API(outputChannel, provider, makeClineProviderFactory()), setValues, log } } @@ -127,4 +140,34 @@ describe("API - configuration", () => { expect(log).toHaveBeenCalledWith(`[setConfiguration] Ignored an invalid value for ${key}`) }) }) + + it("omits host-owned viewStates from the settings the API returns", () => { + // API reads only getValues() in this test; the double provides that plus the members the + // constructor touches (context, on, log) and leaves the rest of ClineProvider unbuilt. + const provider = { + context: {}, + on: vi.fn(), + log: vi.fn(), + getValues: () => ({ + currentApiConfigName: "default", + // Minted by the host: per-view selections plus the claim token that binds them. + // A secret, so the filter has to drop it for the same reason it drops viewStates. + apiKey: "secret-api-key", + viewStates: { "view-1": { claimToken: "claim-token-1", mode: "code" } }, + }), + } as unknown as ClineProvider + const outputChannel = makeOutputChannel() + const api = new API(outputChannel, provider, makeClineProviderFactory()) + + const configuration = api.getConfiguration() + + // Exact output: the portable setting survives and both host-owned keys are gone. + expect(configuration).toEqual({ currentApiConfigName: "default" }) + expect(Object.keys(configuration)).not.toContain("viewStates") + expect("viewStates" in configuration).toBe(false) + // The value must not escape either: the claim token is host-owned state. + // Neither value may escape either: the claim token and the API key are host-owned state. + expect(JSON.stringify(configuration)).not.toContain("claim-token-1") + expect(JSON.stringify(configuration)).not.toContain("secret-api-key") + }) }) diff --git a/src/extension/__tests__/api-start-new-task.spec.ts b/src/extension/__tests__/api-start-new-task.spec.ts index e33f2b3553..7c9de331d2 100644 --- a/src/extension/__tests__/api-start-new-task.spec.ts +++ b/src/extension/__tests__/api-start-new-task.spec.ts @@ -19,7 +19,7 @@ function createProvider(taskId: string) { on: vi.fn(), evictCurrentTask: vi.fn().mockResolvedValue(undefined), postStateToWebview: vi.fn().mockResolvedValue(undefined), - postMessageToWebview: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(true), createTask: vi.fn().mockResolvedValue(task), }) } diff --git a/src/extension/api.ts b/src/extension/api.ts index 623b0492d7..dde92dc914 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -32,6 +32,7 @@ import { Package } from "../shared/package" import type { Mode } from "../shared/modes" import type { ClineProvider } from "../core/webview/ClineProvider" import type { ClineProviderFactory } from "../core/webview/ClineProviderFactory" +import { isNonPortableSettingsKey } from "../core/config/ContextProxy" import { withoutInvalidDisplaySettings } from "../core/config/displaySettings" import { readTaskMessages } from "../core/task-persistence/taskMessages" import { Terminal } from "../integrations/terminal/Terminal" @@ -623,9 +624,17 @@ export class API extends EventEmitter implements RooCodeAPI { // Global Settings Management + /** + * The settings an external caller may read. Secrets are the host's, and so are the per-view + * selections plus the claim token that binds them: a caller that could read viewStates could + * replay another view's identity, so both are dropped here rather than at whichever caller a + * reviewer happened to point at. getValues() still serves the host's own consumers. + */ public getConfiguration(): RooCodeSettings { return Object.fromEntries( - Object.entries(this.sidebarProvider.getValues()).filter(([key]) => !isSecretStateKey(key)), + Object.entries(this.sidebarProvider.getValues()).filter( + ([key]) => !isSecretStateKey(key) && !isNonPortableSettingsKey(key), + ), ) } diff --git a/src/services/language-models/copilotModelBroadcast.ts b/src/services/language-models/copilotModelBroadcast.ts index 000fd6dbdc..c9baf248a0 100644 --- a/src/services/language-models/copilotModelBroadcast.ts +++ b/src/services/language-models/copilotModelBroadcast.ts @@ -10,7 +10,7 @@ export interface CopilotModelBroadcastTarget { type: typeof VsCodeLmModelsMessageType.githubCopilotModels vsCodeLmModels: Awaited> githubCopilotAccount: string | null - }): Promise + }): Promise } export interface CopilotModelBroadcastOptions {