From a3c499d676967eb06413643764a6ad12407cf38d Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:53:53 +0800 Subject: [PATCH 1/9] feat(webview): per-view view-state identity and host-minted claim token (N1) Split unit N1 of #1546: viewId/viewStateId identity, host-minted claim binding, viewStateSchema + viewStateClaim message types. F0/F3 content routed out; load/persistence/buffer remain for N2/N3. --- .../src/__tests__/global-settings.test.ts | 43 ++ packages/types/src/__tests__/index.test.ts | 5 + packages/types/src/global-settings.ts | 17 + packages/types/src/vscode-extension-host.ts | 15 + src/core/config/ProviderSettingsManager.ts | 20 +- .../__tests__/ProviderSettingsManager.spec.ts | 7 +- src/core/webview/ClineProvider.ts | 624 +++++++++++++++++- .../webview/__tests__/ClineProvider.spec.ts | 269 +++++++- 8 files changed, 978 insertions(+), 22 deletions(-) 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 eb51c1d67d..b09bae2635 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -114,6 +114,22 @@ 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 capability token binding this entry to the webview that wrote it. + * Minted by the extension host (`crypto.randomUUID()`), delivered to that webview and + * never accepted from webview input. An entry is only re-adopted for a webview that + * presents this token, so a view cannot read another (e.g. closed) view's selections. + */ + claimToken: z.string().optional(), +}) + /** * GlobalSettings */ @@ -122,6 +138,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 a5393ddc66..bc6e6c6279 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -111,6 +111,7 @@ export interface ExtensionMessage { | "originalContent" | "rooHistoryImportProgress" | "themeFixtureProbeRequest" + | "viewStateClaim" text?: string /** For fileContent: { path, content, error? } */ fileContent?: { path: string; content: string | null; error?: string } @@ -169,6 +170,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 promptText?: string results?: @@ -669,6 +677,13 @@ export interface WebviewMessage { | "openRulesDirectory" | "themeFixtureProbeResponse" text?: string + viewStateId?: string + /** + * For `webviewDidLaunch`: the host-minted claim previously delivered for `viewStateId`. + * The webview only echoes what it was given; the host rejects an id whose stored claim + * does not match, so a view cannot adopt another view's persisted selections. + */ + viewStateClaim?: string taskId?: string editedMessageContent?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud" diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 01695d7994..306253578a 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -55,6 +55,19 @@ export const providerProfilesSchema = z.object({ export type ProviderProfiles = z.infer +/** + * Signals that a profile's configuration no longer exists. Callers that treat an + * already-deleted profile as an idempotent no-op branch on this type instead of + * matching error message text, which a profile name containing the phrase could + * otherwise spoof. + */ +export class ProviderSettingsNotFoundError extends Error { + constructor(message: string) { + super(message) + this.name = "ProviderSettingsNotFoundError" + } +} + export class ProviderSettingsManager { private static readonly SCOPE_PREFIX = "roo_cline_config_" private readonly defaultConfigId = this.generateId() @@ -511,7 +524,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 +535,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__/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/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3d88df632b..ce1cf58bab 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -59,6 +59,7 @@ import { isRetiredProvider, modelIdKeys, providerIdentifiers, + PROVIDER_SETTINGS_KEYS, } from "@roo-code/types" import { RateLimitClock, createRateLimitClock } from "../task/RateLimitClock" import { TaskRegistry } from "../task/TaskRegistry" @@ -112,7 +113,7 @@ import { buildApiHandler } from "../../api" import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../api/providers/fetchers/lmstudio" import { ContextProxy } from "../config/ContextProxy" -import { ProviderSettingsManager } from "../config/ProviderSettingsManager" +import { ProviderSettingsManager, ProviderSettingsNotFoundError } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" import { PendingActionSettlementError, Task } from "../task/Task" @@ -139,6 +140,14 @@ 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 + /** * 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 @@ -219,6 +228,10 @@ export class ClineProvider public static readonly sideBarId = `${Package.name}.SidebarProvider` public static readonly tabPanelId = `${Package.name}.TabPanelProvider` private static activeInstances: Set = new Set() + private static nextViewId = 0 + private static readonly MAX_PERSISTED_VIEW_STATES = 50 + private static readonly MAX_VIEW_STATE_ID_LENGTH = 64 + private static persistedViewStateWriteQueue: Promise = Promise.resolve() private disposables: vscode.Disposable[] = [] private webviewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< @@ -351,6 +364,45 @@ export class ClineProvider */ private clineMessagesSeq = 0 + /** + * Unique identifier for this provider instance's view. + * Based on renderContext and a monotonically increasing counter to ensure uniqueness across multiple instances. + */ + public readonly viewId: string + + /** + * Stable identifier for persisted per-view state keys. + * Defaults to viewId until the webview reports its VS Code-persisted id. + * A durable entry under this id is bound to the webview that wrote it by a host-minted + * claim token (see setViewStateId), so the entry - and the profile its pin brings back + * into the state post - can outlive this instance without becoming adoptable by any + * other webview. + */ + private viewStateId: string + + /** + * Whether this instance wrote an entry under its own session-local temporary id. + * Temporary ids restart from 0 on every extension host start, so an entry under the + * same name can belong to a different session. Only re-key entries this instance wrote. + */ + private wroteUnderTemporaryViewStateId = false + + /** + * Local state buffer for this specific view instance. + * Used to isolate mode, apiConfiguration, and other fields from the shared ContextProxy singleton + * when running in parallel (multi-tab) mode. + */ + private viewLocalState: Partial = {} + + /** + * This view's pinned profile name from the view-local buffer. Exposed for + * sibling-instance inspection (getAllInstances() filtering): `viewLocalState` + * is private and must not be reached through bracket access. + */ + get pinnedProfileName(): string | undefined { + return this.viewLocalState.currentApiConfigName + } + public isViewLaunched = false public settingsImportedAt?: number public readonly latestAnnouncementId = "oct-2026-v3.90.0-providers-tools-reasoning" // v3.90.0 providers and models, safer tool execution, reasoning and streaming @@ -366,14 +418,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.mdmService = mdmService void this.updateGlobalState("codebaseIndexModels", EMBEDDING_MODEL_PROFILES) @@ -520,6 +575,344 @@ export class ClineProvider } } + /** + * Reads the registered viewStates map, returning a defensive copy. + * When fresh is set, the map is read directly from globalState (bypassing the + * ContextProxy cache) so serialized writes never observe a stale in-memory value. + */ + private getPersistedViewStates(options: { fresh?: boolean } = {}): Record { + const viewStates = options.fresh + ? this.context.globalState.get("viewStates") + : this.contextProxy.getValue("viewStates") + + if (!viewStates || typeof viewStates !== "object" || Array.isArray(viewStates)) { + return Object.create(null) + } + + // Copy onto a null prototype. A view id that names an Object.prototype key + // ("constructor", "toString", "valueOf", ...) must not resolve through the + // prototype chain and be mistaken for a persisted entry. + return Object.assign(Object.create(null), viewStates) + } + + /** + * Persists this view's non-secret selections through the serialized write queue. + * The write re-reads the map fresh and merges into the existing entry, removing the + * entry entirely when nothing persistable remains, so concurrent views cannot clobber it. + * The entry is keyed by the view id active when the change was made. Writes captured + * while the provider still holds its temporary (pre-launch) id persist under that id + * and are re-keyed to the stable view id when the webview registers one, so a change + * that lands before the launch message stays durable instead of being lost. + */ + private async savePersistedViewState(values: Partial): Promise { + // Capture the id at change time: a write belongs to the view that was active + // when the change was made, even if a newer id is registered while it is queued. + const viewStateId = this.viewStateId + // Set by the queued write when it had to mint a claim, delivered after it lands. + let mintedClaimToken: string | undefined + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + // The first write to an unowned temporary id claims the slot instead of merging + // into it: temporary names restart from 0 on every host start, so an entry under + // this name can be a leftover from an earlier session, and merging would adopt its + // fields and later re-key them under the stable id. Entries under a stable id, or + // under the temporary id this instance already authored, keep the merge semantics. + const ownsTargetEntry = viewStateId !== this.viewId || this.wroteUnderTemporaryViewStateId + const current = ownsTargetEntry ? (states[viewStateId] ?? {}) : {} + const next: PersistedViewState = { ...current } + + if ("mode" in values) { + if (values.mode === undefined || values.mode === null) { + delete next.mode + } else { + next.mode = values.mode + } + } + + if ("currentApiConfigName" in values) { + if (values.currentApiConfigName === undefined || values.currentApiConfigName === null) { + delete next.currentApiConfigName + } else { + next.currentApiConfigName = values.currentApiConfigName + } + } + + let wroteEntry = false + if (!next.mode && !next.currentApiConfigName) { + delete states[viewStateId] + } else { + next.updatedAt = values.updatedAt ?? Date.now() + // Bind the entry to this webview. An existing entry keeps the token it was + // adopted with (so a reload can still prove ownership); a new entry - or one + // written before claims existed - gets a fresh host-minted token. The token is + // never taken from `values`: it is host-owned, and webview input must not be + // able to choose or supply one. + if (!next.claimToken) { + next.claimToken = crypto.randomUUID() + mintedClaimToken = next.claimToken + } + states[viewStateId] = next + wroteEntry = true + } + + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states)) + // Authorship is recorded only after the durable write lands: a failed persistence + // must not mark the temporary id as owned, or a later load would adopt an entry + // that was never actually stored. + if (wroteEntry && viewStateId === this.viewId) { + this.wroteUnderTemporaryViewStateId = true + } + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + + // Delivered only once the durable write landed: a webview must never hold a claim the + // host did not store, or the next launch would present a token the entry cannot match. + if (mintedClaimToken) { + this.postViewStateClaimToWebview(viewStateId, mintedClaimToken) + } + } + + /** + * Removes the given view's entry from the registered viewStates map. + * Runs through the serialized write queue to avoid racing concurrent view-state writes. + */ + private async clearPersistedViewState(viewStateId = this.viewStateId): Promise { + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + delete states[viewStateId] + await this.contextProxy.setValue("viewStates", states) + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** + * Re-binds a claim-less viewStates entry to this webview with a freshly minted claim. + * Entries written before claims existed cannot be attributed to any webview, so their + * selections are dropped instead of being carried over: keeping them would let the new + * token re-expose them on a later launch, which is exactly the disclosure this binding + * exists to prevent. Runs through the serialized write queue like every other mutation. + */ + private async rebindUnboundViewStateEntry(viewStateId: string): Promise { + const claimToken = crypto.randomUUID() + let rebound = false + + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + const states = this.getPersistedViewStates({ fresh: true }) + const current = states[viewStateId] + // A concurrent registration may have bound this key while this write was queued: + // never overwrite an existing claim, and let the caller refuse the stale adoption. + if (current && typeof current.claimToken === "string" && current.claimToken) { + return + } + states[viewStateId] = { claimToken, updatedAt: Date.now() } + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states)) + rebound = true + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + + return rebound ? claimToken : undefined + } + + /** + * Delivers the host-minted claim that governs a durable viewStates key to this view's + * webview. The webview only stores it and echoes it back on the next launch: it can + * never mint or choose a claim itself. + */ + private postViewStateClaimToWebview(viewStateId: string, claimToken: string): void { + void this.postMessageToWebview({ type: "viewStateClaim", viewStateId, viewStateClaim: claimToken }) + } + + /** + * Keeps only the most recently updated entries of the persisted view states map, + * bounded by MAX_PERSISTED_VIEW_STATES so the global key cannot grow unboundedly. + */ + private prunePersistedViewStates(states: Record): Record { + return Object.fromEntries( + Object.entries(states) + .sort(([, a], [, b]) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)) + .slice(0, ClineProvider.MAX_PERSISTED_VIEW_STATES), + ) + } + + /** + * Re-keys this provider's temporary pre-launch viewStates entry to the newly + * registered stable id so pre-launch writes become durable under the stable key + * instead of orphaning under a session-local temporary id. Only the provider's own + * temporary id is eligible: an entry under a previously registered stable id belongs + * to that webview's storage and is left alone. When the stable entry already exists + * it wins and the temporary entry is dropped, because temporary ids are session + * counters that can collide across window reloads. Runs through the serialized write + * queue like every other viewStates mutation. An entry this instance never wrote is + * left alone, so a stale entry that happens to carry the same session-local name is + * not adopted. + */ + private async rekeyPersistedViewStateEntry(nextViewStateId: string): Promise { + const previousViewStateId = this.viewId + + const write = ClineProvider.persistedViewStateWriteQueue.then(async () => { + // An entry under this name can belong to a different session, because temporary + // ids restart from 0 on every extension host start. Only re-key entries this + // instance wrote, otherwise a stale entry from another session is adopted. + if (!this.wroteUnderTemporaryViewStateId) { + return + } + + const states = this.getPersistedViewStates({ fresh: true }) + const previous = states[previousViewStateId] + + if (!previous) { + return + } + + delete states[previousViewStateId] + + if (!states[nextViewStateId]) { + states[nextViewStateId] = previous + } + + await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states)) + + // Only a successful write clears authorship: a failed attempt leaves the entry + // under the temporary name, and a retry must still be able to re-key it. + this.wroteUnderTemporaryViewStateId = false + }) + + ClineProvider.persistedViewStateWriteQueue = write.catch(() => {}) + await write + } + + /** + * Registers this provider's stable view identifier and loads any persisted selections it owns. + * The identifier is sanitized so it remains a safe object key in the shared viewStates map. + * + * `presentedClaim` is the host-minted token this webview was given for that id. It is the + * only proof of ownership: an entry that carries a claim is loaded only when the presented + * token matches it, so a webview cannot adopt another (e.g. disposed, still durable) view's + * id and read that view's pinned profile - and the profile secrets copied into the state + * post - back out of it. The check runs before any re-keying or loading. + */ + public async setViewStateId(viewStateId: string | undefined, presentedClaim?: string): Promise { + // The webview generates this id on purpose - it has to survive a webview reload, + // which recreates the view on both sides - so the host cannot mint it instead. + // What the host owns is which view may hold a given persisted key, so everything + // below validates the supplied value before it becomes one. + if (typeof viewStateId !== "string") { + // A crafted postMessage can carry any JSON value here even though the message + // type declares a string; the persisted key must stay a string. + this.log(`[setViewStateId] Ignoring a non-string viewStateId for view ${this.viewId}.`) + return + } + + const normalizedViewStateId = viewStateId.trim().replace(/[^A-Za-z0-9_-]/g, "_") + + if ( + !normalizedViewStateId || + normalizedViewStateId === this.viewStateId || + // Reject "__proto__" and any other prototype-chain key. The map is copied + // onto a null prototype, so these names can no longer resolve through the + // chain, but they are still rejected as ids so a caller cannot store an + // entry under a name that looks like an inherited member. + normalizedViewStateId === "__proto__" + ) { + return + } + + // Bound the key: the webview generates a UUID, so a much longer value is not a + // view id, and an unbounded key is unbounded growth in the shared viewStates map. + if (normalizedViewStateId.length > ClineProvider.MAX_VIEW_STATE_ID_LENGTH) { + this.log( + `[setViewStateId] Ignoring an oversized viewStateId (${normalizedViewStateId.length} chars) for view ${this.viewId}.`, + ) + return + } + + const claimant = ClineProvider.getAllInstances().find( + (instance) => instance !== this && instance.viewStateId === normalizedViewStateId, + ) + if (claimant) { + // Ownership check: a key another live view holds must not be adopted. Doing so + // would load that view's pinned profile, nested overlay, and mode into this + // provider, and this webview could then overwrite that view's persisted entry. + // Staying on the temporary id keeps this view working against the shared state. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: already claimed by live view ${claimant.viewId}.`, + ) + return + } + + // Claim binding, checked before any re-key or load so a refused registration can + // neither read nor mutate a persisted entry. Read fresh from storage rather than + // through the ContextProxy cache: a security check must not trust a stale copy. + const persistedEntry = this.getPersistedViewStates({ fresh: true })[normalizedViewStateId] + + if (persistedEntry) { + const storedClaim = persistedEntry.claimToken + + if (typeof storedClaim !== "string" || !storedClaim) { + // A pre-claim entry cannot be attributed to any webview, so its selections are + // never loaded. Re-establish the binding with a fresh host-minted claim and hand + // that claim to this webview: the key becomes adoptable again (no upgrade brick) + // and the unbound selections are gone, so the new token cannot resurrect them. + const reboundClaim = await this.rebindUnboundViewStateEntry(normalizedViewStateId) + + if (reboundClaim === undefined) { + // A concurrent registration bound the key while the re-bind was queued: this + // launch adopts a key another webview now owns, so refuse it instead of + // handing out a claim that is not ours. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: bound concurrently by another registration.`, + ) + return + } + + this.postViewStateClaimToWebview(normalizedViewStateId, reboundClaim) + this.log( + `[setViewStateId] Re-bound claim-less viewStates entry ${normalizedViewStateId} for view ${this.viewId}: unbound selections were not loaded.`, + ) + } else if (typeof presentedClaim !== "string" || presentedClaim !== storedClaim) { + // Fail closed: the entry belongs to a webview that never presented this view's + // claim. Rejection is side-effect free, so presenting an id cannot probe, mutate + // or delete another view's entry. + this.log( + `[setViewStateId] Refusing viewStateId ${normalizedViewStateId} for view ${this.viewId}: claim missing or mismatched.`, + ) + return + } + } + + const previousViewStateId = this.viewStateId + + this.viewStateId = normalizedViewStateId + + try { + // Re-key any durable entry written under the temporary pre-launch id before + // loading, so the load sees the view's own pre-registration selections. + await this.rekeyPersistedViewStateEntry(this.viewStateId) + + // Re-affirm the binding for the id just adopted: a claim minted while this view was + // still on its temporary id now governs this key (the entry was re-keyed), so the + // webview is told the canonical (viewStateId, claim) pair to echo on its next launch. + const adoptedClaim = this.getPersistedViewStates({ fresh: true })[this.viewStateId]?.claimToken + + if (adoptedClaim) { + this.postViewStateClaimToWebview(this.viewStateId, adoptedClaim) + } + } catch (error) { + // A persistence failure must not leave the provider holding an id that was + // never registered: restore the previous id so a later launch retries the + // registration and the load instead of the guard above early-returning for + // the failed id. + this.viewStateId = previousViewStateId + throw error + } + } + /** * Override EventEmitter's on method to match TaskProviderLike interface */ @@ -847,7 +1240,6 @@ export class ClineProvider ) } } - async dispose() { if (this._disposed) { return @@ -1232,7 +1624,6 @@ export class ClineProvider ) return run } - private async createTaskWithHistoryItemUnlocked( historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }, options?: { startTask?: boolean }, @@ -1469,7 +1860,6 @@ export class ClineProvider return task } - public async postMessageToWebview(message: ExtensionMessage) { if (this._disposed) { return @@ -1725,7 +2115,6 @@ export class ClineProvider this.handleModeSwitchUnlocked(newMode, targetTask, signal), ) } - private async handleModeSwitchUnlocked( newMode: Mode, targetTask: Task | null | undefined, @@ -1999,7 +2388,6 @@ export class ClineProvider ) } } - private async persistProviderProfile( name: string, providerSettings: ProviderSettings, @@ -2060,7 +2448,6 @@ export class ClineProvider await this.postStateToWebview() return id } - async deleteProviderProfile(profileToDelete: ProviderSettingsEntry) { const globalSettings = this.contextProxy.getValues() let profileToActivate: string | undefined = globalSettings.currentApiConfigName @@ -2126,7 +2513,6 @@ export class ClineProvider this.activateProviderProfileUnlocked(args, options, signal), ) } - private async activateProviderProfileUnlocked( args: { name: string } | { id: string }, options?: { @@ -2249,9 +2635,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. @@ -3301,7 +3684,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) } @@ -3309,17 +3691,12 @@ 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(values) } - - // dev - async resetState() { const answer = await vscode.window.showInformationMessage( t("common:confirmation.reset_state"), @@ -4651,4 +5028,213 @@ 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: ViewLocalStateValues[K] | undefined, + ): Promise { + await this._saveViewLocalStateFromMutation({ [key]: value } as ViewLocalStateValues) + + this.log(`[saveViewState] Saved ${String(key)} for viewId ${this.viewId}`) + } + + /** + * Persists the view-local subset of a ContextProxy mutation, then updates the in-memory + * viewLocalState buffer. Persistence is awaited first so a failed durable write cannot + * leave the local cache ahead of the persisted state. + */ + private async _saveViewLocalStateFromMutation( + values: Partial & Partial, + ): Promise { + await this._persistViewLocalStateFromMutation(values) + this._updateViewLocalStateFromMutation(values) + } + + /** + * Update or invalidate viewLocalState when ContextProxy is mutated via setValues, setValue, + * profile upsert/activation/deletion, or resetState. This ensures the local cache stays in + * sync with global state changes that would otherwise be invisible behind mergedStateValues. + */ + private _updateViewLocalStateFromMutation(values: Partial & Partial): void { + if ("mode" in values) { + const val = values.mode + if (val === undefined || val === null) { + delete this.viewLocalState.mode + } else { + this.viewLocalState.mode = val + } + } + + if ("currentApiConfigName" in values) { + const val = values.currentApiConfigName + if (val === undefined || val === null) { + delete this.viewLocalState.currentApiConfigName + } else { + this.viewLocalState.currentApiConfigName = val + } + } + + if ("apiConfiguration" in values) { + const val = values.apiConfiguration + if (val === undefined || val === null) { + delete this.viewLocalState.apiConfiguration + } else { + this.viewLocalState.apiConfiguration = val + } + } 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) { + await this.savePersistedViewState(persistedValues) + } + } + + /** + * Clear view-local state cache so that getState() falls back to ContextProxy defaults. + */ + private _clearViewLocalState(): void { + this.viewLocalState = {} + // Load initial state from global state into viewLocalState buffer after dependencies used by getState are ready. + void this.loadViewState() + } + + /** + * Loads non-secret persisted selections from the registered viewStates map. + * Missing entries are intentionally left unset so getState() falls back to shared ContextProxy values. + * Fields mutated while the async profile lookup is in flight are reapplied on top of the + * loaded state, field by field, so in-flight user selections are not clobbered by the load. + */ + private async loadViewState(): Promise { + // 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 preLoadBuffer = { ...this.viewLocalState } + + if (persisted?.mode) { + // A persisted mode may reference a custom mode that was deleted after it was + // pinned: restore it only when the slug still resolves, so a stale slug cannot + // shadow the shared mode from getState(). + const customModes = await this.customModesManager.getCustomModes() + if (getModeBySlug(persisted.mode, customModes)) { + loadedState.mode = persisted.mode as Mode + } else { + this.log(`[loadViewState] Ignoring unknown persisted mode "${persisted.mode}"`) + } + } + + if (persisted?.currentApiConfigName) { + loadedState.currentApiConfigName = persisted.currentApiConfigName + + try { + const { name: _name, ...apiConfiguration } = await this.providerSettingsManager.getProfile({ + name: persisted.currentApiConfigName, + }) + loadedState.apiConfiguration = apiConfiguration as ProviderSettings + } catch (error) { + this.log( + `[loadViewState] Unable to resolve API profile '${persisted.currentApiConfigName}' for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + + if (this.viewStateId !== loadedForViewId) { + this.log(`[loadViewState] Discarding stale state for superseded view id ${loadedForViewId}`) + return + } + + // Reapply the buffer fields that changed while the load was in flight, + // tracking the change instead of testing against undefined: a field cleared + // during the load window must stay cleared (the loaded value must not + // resurrect it), and a field written to a new value must win over it. + // Untouched fields keep the persisted values authoritative, and the + // pre-load buffer is never merged wholesale so stale temporary-id state + // cannot override the stable persisted state. + const postLoadBuffer = this.viewLocalState + const mergedState: Partial = { ...loadedState } + + if (!Object.is(preLoadBuffer.mode, postLoadBuffer.mode)) { + if (postLoadBuffer.mode === undefined) { + delete mergedState.mode + } else { + mergedState.mode = postLoadBuffer.mode + } + } + + if (!Object.is(preLoadBuffer.currentApiConfigName, postLoadBuffer.currentApiConfigName)) { + if (postLoadBuffer.currentApiConfigName === undefined) { + delete mergedState.currentApiConfigName + } else { + mergedState.currentApiConfigName = postLoadBuffer.currentApiConfigName + } + } + + if (!Object.is(preLoadBuffer.apiConfiguration, postLoadBuffer.apiConfiguration)) { + if (postLoadBuffer.apiConfiguration === undefined) { + delete mergedState.apiConfiguration + } else { + mergedState.apiConfiguration = postLoadBuffer.apiConfiguration + } + } + + this.viewLocalState = mergedState + this.log(`[loadViewState] Loaded state for viewId ${this.viewId}`) + } catch (error) { + this.log( + `[loadViewState] Error loading state for viewId ${this.viewId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 93543435e2..8b12a20061 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1,4 +1,4 @@ -// pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts +// pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts import fs from "fs" import * as path from "path" @@ -3579,6 +3579,273 @@ describe("ClineProvider", () => { ]) }) }) + + describe("setViewStateId", () => { + it('should ignore "__proto__" and keep the temporary viewId', async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + await provider["setViewStateId"]("__proto__") + + // "__proto__" is rejected before assignment so a per-view entry can never be + // keyed through the Object.prototype setter: the temporary id stays active and + // nothing is persisted under the reserved name. + expect(provider["viewStateId"]).toBe(provider.viewId) + expect(mockContext.globalState.get("viewStates")).toBeUndefined() + expect(provider["viewLocalState"]).toEqual({}) + + await provider.dispose() + }) + + it("restores the previous view id when the registration write fails so a later launch retries", async () => { + const contextProxy = new ContextProxy(mockContext) + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + contextProxy, + new WebviewFocusTracker(), + ) + // Seed a pre-launch entry under the temporary id through the real write path, + // so the re-key has real work to do and records this instance as its author. + await provider["savePersistedViewState"]({ mode: "architect", updatedAt: 1 }) + + const setValueSpy = vi.spyOn(contextProxy, "setValue").mockRejectedValue(new Error("storage down")) + + await expect(provider["setViewStateId"]("stable-sidebar-view")).rejects.toThrow("storage down") + + // The failed id must not stick: the provider keeps its previous (temporary) + // id so a later launch retries registration and the load instead of the + // guard early-returning for an id that was never persisted. + expect(provider["viewStateId"]).toBe(provider.viewId) + + // A later retry succeeds once the storage write works again, and the + // pre-launch entry lands under the registered id. + setValueSpy.mockRestore() + await provider["setViewStateId"]("stable-sidebar-view") + expect(provider["viewStateId"]).toBe("stable-sidebar-view") + expect(mockContext.globalState.get("viewStates")).toEqual({ + "stable-sidebar-view": { mode: "architect", updatedAt: 1, claimToken: expect.any(String) }, + }) + + await provider.dispose() + }) + }) + + describe("view state claim binding", () => { + it("refuses a viewStateId bound by a concurrent registration through setViewStateId", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + const beforeViewStateId = provider["viewStateId"] + const postSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + // A claim-less entry is adoptable at the first read, but a concurrent registration + // binds the key while this launch's queued re-bind is waiting its turn. + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + await provider["setViewStateId"]("legacy-view") + + // The launch must not adopt a key another webview now owns: no claim is delivered, + // the provider stays on its temporary id, and the winner's claim is untouched. + expect(postSpy).not.toHaveBeenCalledWith(expect.objectContaining({ type: "viewStateClaim" })) + expect(provider["viewStateId"]).toBe(beforeViewStateId) + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + postSpy.mockRestore() + await provider.dispose() + }) + + it("does not overwrite a claim bound by a concurrent registration", async () => { + const provider = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + await provider.contextProxy.setValue("viewStates", { "legacy-view": { updatedAt: 1 } }) + // A concurrent registration binds this key while the re-bind write is queued. + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve().then(async () => { + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { claimToken: "winner", updatedAt: 2 }, + }) + }) + + const claim = await provider["rebindUnboundViewStateEntry"]("legacy-view") + + // The queued re-bind must not replace the claim the other registration wrote; it + // reports that nothing was rebound so the caller can refuse the stale adoption. + expect(claim).toBeUndefined() + const states = provider.contextProxy.getValue("viewStates") as Record + expect(states["legacy-view"].claimToken).toBe("winner") + ClineProvider["persistedViewStateWriteQueue"] = Promise.resolve() + await provider.dispose() + }) + + const createProvider = () => + new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + + const storedEntries = () => + mockContext.globalState.get>("viewStates") ?? {} + + test("refuses a disposed view's id when no claim is presented, and loads nothing", async () => { + // The vulnerable path: a view registers a durable entry, is disposed, and a second + // webview presents the same id. dispose() removes the instance but leaves the entry, + // so only the claim can stop the second webview from loading the first one's pinned + // profile (and the profile's secrets) through the state post. + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("mode", "architect") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + + const storedClaim = storedEntries()["closed-view"]?.claimToken + expect(storedClaim).toEqual(expect.any(String)) + + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + await intruder["setViewStateId"]("closed-view") + + // Fail closed: the id is not adopted, no selections reach this view and no state is + // posted back to the webview that asked for it. + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect((await intruder.getState()).currentApiConfigName).not.toBe("profile-with-secrets") + expect(postMessageSpy).not.toHaveBeenCalled() + + // A refused adoption is side-effect free: presenting an id must not probe or rewrite + // the entry that a different webview owns. + expect(storedEntries()["closed-view"]).toMatchObject({ + mode: "architect", + currentApiConfigName: "profile-with-secrets", + claimToken: storedClaim, + }) + + await intruder.dispose() + }) + + test("refuses a mismatched claim", async () => { + const owner = createProvider() + await owner["setViewStateId"]("closed-view") + await owner.saveViewState("currentApiConfigName", "profile-with-secrets") + await owner.dispose() + + const intruder = createProvider() + const postMessageSpy = vi.spyOn(intruder, "postMessageToWebview").mockResolvedValue(undefined) + + // A webview-generated value is not a claim: only the token the host minted matches. + await intruder["setViewStateId"]("closed-view", "webview-chosen-claim") + + expect(intruder["viewStateId"]).toBe(intruder.viewId) + expect(intruder["viewLocalState"]).toStrictEqual({}) + expect(postMessageSpy).not.toHaveBeenCalled() + + await intruder.dispose() + }) + + test("does not churn the claim on later writes to the same entry", async () => { + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + await provider["setViewStateId"]("minted-view") + await provider.saveViewState("mode", "architect") + + const mintedClaim = storedEntries()["minted-view"]?.claimToken + expect(mintedClaim).toEqual(expect.any(String)) + + await provider.saveViewState("currentApiConfigName", "profile-b") + + // A merge into an entry this view already owns keeps its token, so the claim the + // webview holds keeps working and is not re-sent. + expect(storedEntries()["minted-view"]?.claimToken).toBe(mintedClaim) + expect( + postMessageSpy.mock.calls.map(([message]) => message).filter((m) => m.type === "viewStateClaim"), + ).toHaveLength(1) + }) + + test("re-binds a claim-less legacy entry instead of loading its selections", async () => { + // An entry written before claims existed cannot be attributed to any webview: its + // selections are refused, and the key is re-bound with a fresh host-minted claim so + // the upgrade does not brick per-view persistence. + await provider.contextProxy.setValue("viewStates", { + "legacy-view": { mode: "architect", currentApiConfigName: "profile-a", updatedAt: 1 }, + }) + + const postMessageSpy = vi.spyOn(provider, "postMessageToWebview").mockResolvedValue(undefined) + + await provider["setViewStateId"]("legacy-view") + + // No stored selection is adopted... + expect(provider["viewStateId"]).toBe("legacy-view") + expect(provider["viewLocalState"]).toStrictEqual({}) + + // ...the old entry is replaced by a claim-bearing one with the unbound selections gone... + const rebound = storedEntries()["legacy-view"] + expect(rebound).toEqual({ claimToken: expect.any(String), updatedAt: expect.any(Number) }) + expect(rebound).not.toHaveProperty("mode") + expect(rebound).not.toHaveProperty("currentApiConfigName") + + // ...and the fresh claim is handed to this webview so the next launch can prove ownership. + expect(postMessageSpy).toHaveBeenCalledWith({ + type: "viewStateClaim", + viewStateId: "legacy-view", + viewStateClaim: rebound?.claimToken, + }) + + await provider.dispose() + + const reopened = createProvider() + await reopened["setViewStateId"]("legacy-view", rebound?.claimToken) + + expect(reopened["viewStateId"]).toBe("legacy-view") + expect(reopened["viewLocalState"]).toStrictEqual({}) + + await reopened.dispose() + }) + + test("still refuses an id a live view holds, even when the claim matches", async () => { + const owner = createProvider() + await owner["setViewStateId"]("shared-live-view") + await owner.saveViewState("mode", "architect") + const claim = storedEntries()["shared-live-view"]?.claimToken + + const sibling = createProvider() + + try { + await sibling["setViewStateId"]("shared-live-view", claim) + + // The live-instance guard runs first and keeps working: a second live view never + // shares a key with the view that holds it. + expect(sibling["viewStateId"]).toBe(sibling.viewId) + expect(sibling["viewLocalState"]).toStrictEqual({}) + expect(owner["viewStateId"]).toBe("shared-live-view") + } finally { + await sibling.dispose() + await owner.dispose() + } + }) + }) }) describe("webviewMessageHandler no-floating-promises coverage", () => { From c3595a1f0898988d1e38e920a21d97eaab70c9d7 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:11:42 +0800 Subject: [PATCH 2/9] chore: strip BOM and fix lint imports --- src/core/webview/ClineProvider.ts | 2 +- src/core/webview/__tests__/ClineProvider.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 1e2afb76ba..47d86d4e52 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -113,7 +113,7 @@ import { buildApiHandler } from "../../api" import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../api/providers/fetchers/lmstudio" import { ContextProxy } from "../config/ContextProxy" -import { ProviderSettingsManager, ProviderSettingsNotFoundError } from "../config/ProviderSettingsManager" +import { ProviderSettingsManager } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" import { PendingActionSettlementError, Task } from "../task/Task" diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index aa4da7a56e..521bf33f8a 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1,4 +1,4 @@ -// pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts +// pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts import fs from "fs" import * as path from "path" From 60c5400f203c761538bd327cd46b63d81b55cdd6 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:56:46 +0800 Subject: [PATCH 3/9] fix(webview): address review findings on view-state durability - omit viewStates (host-minted claim tokens) from settings export - keep a pre-launch selection when registration re-keys onto an existing stable entry - narrow saveViewState keys to the durable per-view subset - skip rollback when another provider wrote a newer shared value - restore the ContextProxy cache when globalState.update rejects - clarify that the claim is enforced by setViewStateId; launch wiring follows --- packages/types/src/vscode-extension-host.ts | 6 +++-- src/core/config/ContextProxy.ts | 3 +++ src/core/webview/ClineProvider.ts | 24 +++++++++++++++--- .../webview/__tests__/ClineProvider.spec.ts | 25 +++++++++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 233fa7538f..c4e7788928 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -681,8 +681,10 @@ export interface WebviewMessage { viewStateId?: string /** * For `webviewDidLaunch`: the host-minted claim previously delivered for `viewStateId`. - * The webview only echoes what it was given; the host rejects an id whose stored claim - * does not match, so a view cannot adopt another view's persisted selections. + * The webview only echoes what it was given; the host enforces the match in + * `ClineProvider.setViewStateId` when a view registers this id, so a view cannot adopt + * another view's persisted selections. Forwarding these fields from the launch handler + * is wired by the follow-up launch unit. */ viewStateClaim?: string taskId?: string diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index 1250a7baca..28301337e4 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -36,6 +36,9 @@ 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, }) export class ContextProxy { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 47d86d4e52..33c02465f1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -148,6 +148,13 @@ type PersistedViewState = NonNullable[string] */ type ViewLocalStateValues = Partial & Partial +/** + * The subset of view-local values that has a durable per-view record. `saveViewState` + * and the mutation path handle exactly these keys, so the generic stays narrow enough + * for the compiler to reject e.g. `writeDelayMs`. + */ +type PersistedViewStateValues = Pick + /** * https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts * https://github.com/KumarVariable/vscode-extension-sidebar-html/blob/master/src/customSidebarViewProvider.ts @@ -771,8 +778,19 @@ export class ClineProvider delete states[previousViewStateId] - if (!states[nextViewStateId]) { + const existingStableEntry = states[nextViewStateId] + + if (!existingStableEntry) { states[nextViewStateId] = previous + } else { + // Registration can bind a claim-less legacy key just before this pre-launch entry + // is re-keyed. Keep the selections the stable entry already carries, but fill in + // anything it lacks so a mode chosen before launch is not silently dropped. + states[nextViewStateId] = { + ...previous, + ...existingStableEntry, + updatedAt: Math.max(previous.updatedAt ?? 0, existingStableEntry.updatedAt ?? 0), + } } await this.contextProxy.setValue("viewStates", this.prunePersistedViewStates(states)) @@ -5119,9 +5137,9 @@ export class ClineProvider * 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( + public async saveViewState( key: K, - value: ViewLocalStateValues[K] | undefined, + value: PersistedViewStateValues[K] | undefined, ): Promise { await this._saveViewLocalStateFromMutation({ [key]: value } as ViewLocalStateValues) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 521bf33f8a..036f1f535c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3845,6 +3845,31 @@ describe("ClineProvider", () => { } }) }) + + 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() + }) }) describe("webviewMessageHandler no-floating-promises coverage", () => { From 1b9e8f8bbff90eb26f77e5d6d774490bf281eebb Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:23:27 +0800 Subject: [PATCH 4/9] chore: tighten the view-identity comments --- packages/types/src/global-settings.ts | 6 +- packages/types/src/vscode-extension-host.ts | 5 +- src/core/config/ProviderSettingsManager.ts | 7 +-- src/core/webview/ClineProvider.ts | 63 ++++----------------- 4 files changed, 16 insertions(+), 65 deletions(-) diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index b09bae2635..3da73ee0fe 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -122,10 +122,8 @@ export const viewStateSchema = z.object({ currentApiConfigName: z.string().optional(), updatedAt: z.number().optional(), /** - * Host-minted capability token binding this entry to the webview that wrote it. - * Minted by the extension host (`crypto.randomUUID()`), delivered to that webview and - * never accepted from webview input. An entry is only re-adopted for a webview that - * presents this token, so a view cannot read another (e.g. closed) view's selections. + * Host-minted capability token binding this entry to the webview that wrote it; never + * accepted from webview input and only a matching token re-adopts the entry. */ claimToken: z.string().optional(), }) diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index c4e7788928..c77dbb14c4 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -681,10 +681,7 @@ export interface WebviewMessage { viewStateId?: string /** * For `webviewDidLaunch`: the host-minted claim previously delivered for `viewStateId`. - * The webview only echoes what it was given; the host enforces the match in - * `ClineProvider.setViewStateId` when a view registers this id, so a view cannot adopt - * another view's persisted selections. Forwarding these fields from the launch handler - * is wired by the follow-up launch unit. + * Enforced by `ClineProvider.setViewStateId`; launch forwarding ships in the wiring unit. */ viewStateClaim?: string taskId?: string diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 306253578a..07a5fc3d21 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -55,12 +55,7 @@ export const providerProfilesSchema = z.object({ export type ProviderProfiles = z.infer -/** - * Signals that a profile's configuration no longer exists. Callers that treat an - * already-deleted profile as an idempotent no-op branch on this type instead of - * matching error message text, which a profile name containing the phrase could - * otherwise spoof. - */ +/** Already-deleted profile: callers branch on this type instead of matching message text. */ export class ProviderSettingsNotFoundError extends Error { constructor(message: string) { super(message) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 33c02465f1..b4de7459d6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -377,12 +377,8 @@ export class ClineProvider public readonly viewId: string /** - * Stable identifier for persisted per-view state keys. - * Defaults to viewId until the webview reports its VS Code-persisted id. - * A durable entry under this id is bound to the webview that wrote it by a host-minted - * claim token (see setViewStateId), so the entry - and the profile its pin brings back - * into the state post - can outlive this instance without becoming adoptable by any - * other webview. + * Stable identifier for persisted per-view state keys; binds a durable entry to the + * webview that wrote it through the host-minted claim token. */ private viewStateId: string @@ -602,13 +598,8 @@ export class ClineProvider } /** - * Persists this view's non-secret selections through the serialized write queue. - * The write re-reads the map fresh and merges into the existing entry, removing the - * entry entirely when nothing persistable remains, so concurrent views cannot clobber it. - * The entry is keyed by the view id active when the change was made. Writes captured - * while the provider still holds its temporary (pre-launch) id persist under that id - * and are re-keyed to the stable view id when the webview registers one, so a change - * that lands before the launch message stays durable instead of being lost. + * Persist the non-secret selections through the serialized write queue, re-keying a + * pre-launch write to the stable id once the webview registers one. */ private async savePersistedViewState(values: Partial): Promise { // Capture the id at change time: a write belongs to the view that was active @@ -696,11 +687,8 @@ export class ClineProvider } /** - * Re-binds a claim-less viewStates entry to this webview with a freshly minted claim. - * Entries written before claims existed cannot be attributed to any webview, so their - * selections are dropped instead of being carried over: keeping them would let the new - * token re-expose them on a later launch, which is exactly the disclosure this binding - * exists to prevent. Runs through the serialized write queue like every other mutation. + * Re-bind a claim-less viewStates entry with a freshly minted claim; its unowned + * selections are dropped so the new token cannot re-expose them. */ private async rebindUnboundViewStateEntry(viewStateId: string): Promise { const claimToken = crypto.randomUUID() @@ -746,18 +734,7 @@ export class ClineProvider ) } - /** - * Re-keys this provider's temporary pre-launch viewStates entry to the newly - * registered stable id so pre-launch writes become durable under the stable key - * instead of orphaning under a session-local temporary id. Only the provider's own - * temporary id is eligible: an entry under a previously registered stable id belongs - * to that webview's storage and is left alone. When the stable entry already exists - * it wins and the temporary entry is dropped, because temporary ids are session - * counters that can collide across window reloads. Runs through the serialized write - * queue like every other viewStates mutation. An entry this instance never wrote is - * left alone, so a stale entry that happens to carry the same session-local name is - * not adopted. - */ + /** Re-key this instance's authored pre-launch entry to the stable id. */ private async rekeyPersistedViewStateEntry(nextViewStateId: string): Promise { const previousViewStateId = this.viewId @@ -805,14 +782,8 @@ export class ClineProvider } /** - * Registers this provider's stable view identifier and loads any persisted selections it owns. - * The identifier is sanitized so it remains a safe object key in the shared viewStates map. - * - * `presentedClaim` is the host-minted token this webview was given for that id. It is the - * only proof of ownership: an entry that carries a claim is loaded only when the presented - * token matches it, so a webview cannot adopt another (e.g. disposed, still durable) view's - * id and read that view's pinned profile - and the profile secrets copied into the state - * post - back out of it. The check runs before any re-keying or loading. + * Register the stable view id and load its persisted selections. An entry that carries a + * claim is loaded only when the presented host-minted token matches it. */ public async setViewStateId(viewStateId: string | undefined, presentedClaim?: string): Promise { // The webview generates this id on purpose - it has to survive a webview reload, @@ -5239,12 +5210,7 @@ export class ClineProvider void this.loadViewState() } - /** - * Loads non-secret persisted selections from the registered viewStates map. - * Missing entries are intentionally left unset so getState() falls back to shared ContextProxy values. - * Fields mutated while the async profile lookup is in flight are reapplied on top of the - * loaded state, field by field, so in-flight user selections are not clobbered by the load. - */ + /** 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. @@ -5298,13 +5264,8 @@ export class ClineProvider return } - // Reapply the buffer fields that changed while the load was in flight, - // tracking the change instead of testing against undefined: a field cleared - // during the load window must stay cleared (the loaded value must not - // resurrect it), and a field written to a new value must win over it. - // Untouched fields keep the persisted values authoritative, and the - // pre-load buffer is never merged wholesale so stale temporary-id state - // cannot override the stable persisted state. + // 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 } From a81e24ef7ddad7c42cb88009ef570b20c1d98ad8 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:55:33 +0800 Subject: [PATCH 5/9] fix(webview): load persisted selections when a view id is registered --- src/core/webview/ClineProvider.ts | 13 ++++------- .../webview/__tests__/ClineProvider.spec.ts | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 37a8c51cbf..8425669484 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -891,6 +891,10 @@ export class ClineProvider if (adoptedClaim) { this.postViewStateClaimToWebview(this.viewStateId, adoptedClaim) } + + // Registration is only complete once the view's own persisted selections are in the + // buffer; nothing else loads them now that reset/import handling lives downstream. + 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 @@ -5243,15 +5247,6 @@ export class ClineProvider } } - /** - * Clear view-local state cache so that getState() falls back to ContextProxy defaults. - */ - private _clearViewLocalState(): void { - this.viewLocalState = {} - // Load initial state from global state into viewLocalState buffer after dependencies used by getState are ready. - void this.loadViewState() - } - /** 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 diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 036f1f535c..60e799035e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3601,6 +3601,29 @@ describe("ClineProvider", () => { 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" }, + }) + const logSpy = vi.spyOn(provider, "log") + + await provider["setViewStateId"]("stable-view", "host-token") + + // Registration is what loads a view's own selections: the buffer is populated and + // the load is logged, so a reload restores the pinned mode instead of the shared one. + expect(provider["viewLocalState"].mode).toBe("architect") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Loaded state for viewId")) + + 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( From c9f462682caa2a6603eeb815d7c6d37dce856d3b Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:56:52 +0800 Subject: [PATCH 6/9] chore: tighten the shared view-state docs --- packages/types/src/global-settings.ts | 5 +--- packages/types/src/vscode-extension-host.ts | 5 +--- src/core/webview/ClineProvider.ts | 23 ++++--------------- .../webview/__tests__/ClineProvider.spec.ts | 6 +---- 4 files changed, 8 insertions(+), 31 deletions(-) diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 3da73ee0fe..f9fe1f1139 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -121,10 +121,7 @@ export const viewStateSchema = z.object({ mode: z.string().optional(), currentApiConfigName: z.string().optional(), updatedAt: z.number().optional(), - /** - * Host-minted capability token binding this entry to the webview that wrote it; never - * accepted from webview input and only a matching token re-adopts the entry. - */ + /** Host-minted token binding this entry to the webview that wrote it. */ claimToken: z.string().optional(), }) diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index c77dbb14c4..3954246e15 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -679,10 +679,7 @@ export interface WebviewMessage { | "themeFixtureProbeResponse" text?: string viewStateId?: string - /** - * For `webviewDidLaunch`: the host-minted claim previously delivered for `viewStateId`. - * Enforced by `ClineProvider.setViewStateId`; launch forwarding ships in the wiring unit. - */ + /** For `webviewDidLaunch`: the claim delivered for `viewStateId`, enforced by setViewStateId. */ viewStateClaim?: string taskId?: string editedMessageContent?: string diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8425669484..2d61343b4f 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -376,10 +376,7 @@ export class ClineProvider */ public readonly viewId: string - /** - * Stable identifier for persisted per-view state keys; binds a durable entry to the - * webview that wrote it through the host-minted claim token. - */ + /** Stable id for persisted per-view state keys, bound by the host-minted claim. */ private viewStateId: string /** @@ -597,10 +594,7 @@ export class ClineProvider return Object.assign(Object.create(null), viewStates) } - /** - * Persist the non-secret selections through the serialized write queue, re-keying a - * pre-launch write to the stable id once the webview registers one. - */ + /** 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. @@ -686,10 +680,7 @@ export class ClineProvider await write } - /** - * Re-bind a claim-less viewStates entry with a freshly minted claim; its unowned - * selections are dropped so the new token cannot re-expose them. - */ + /** 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 @@ -781,10 +772,7 @@ export class ClineProvider await write } - /** - * Register the stable view id and load its persisted selections. An entry that carries a - * claim is loaded only when the presented host-minted token matches it. - */ + /** Register the stable view id and load its persisted selections; a claim must match. */ public async setViewStateId(viewStateId: string | undefined, presentedClaim?: string): Promise { // The webview generates this id on purpose - it has to survive a webview reload, // which recreates the view on both sides - so the host cannot mint it instead. @@ -892,8 +880,7 @@ export class ClineProvider this.postViewStateClaimToWebview(this.viewStateId, adoptedClaim) } - // Registration is only complete once the view's own persisted selections are in the - // buffer; nothing else loads them now that reset/import handling lives downstream. + // 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 diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 60e799035e..83eea4a6ed 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3124,8 +3124,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", @@ -3616,10 +3614,8 @@ describe("ClineProvider", () => { await provider["setViewStateId"]("stable-view", "host-token") - // Registration is what loads a view's own selections: the buffer is populated and - // the load is logged, so a reload restores the pinned mode instead of the shared one. + // Registration is what loads a view's own selections. expect(provider["viewLocalState"].mode).toBe("architect") - expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Loaded state for viewId")) await provider.dispose() }) From c9b7fda3abf6d43d2af7368d7413eea89e373a86 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:13:29 +0800 Subject: [PATCH 7/9] fix(webview): track buffer writes so a same-value save survives the load --- src/core/webview/ClineProvider.ts | 18 +++++++++++++++--- .../webview/__tests__/ClineProvider.spec.ts | 2 ++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 2d61343b4f..7e60740ebb 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -393,6 +393,10 @@ export class ClineProvider */ 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 } + /** * This view's pinned profile name from the view-local buffer. Exposed for * sibling-instance inspection (getAllInstances() filtering): `viewLocalState` @@ -5167,7 +5171,14 @@ export class ClineProvider * 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) { const val = values.mode if (val === undefined || val === null) { @@ -5255,6 +5266,7 @@ export class ClineProvider // mutation paths update viewLocalState in place, so a shallow copy is // what makes fields mutated during the load window observable below. const preLoadBuffer = { ...this.viewLocalState } + const preLoadVersions = { ...this.viewLocalStateWriteVersions } if (persisted?.mode) { // A persisted mode may reference a custom mode that was deleted after it was @@ -5293,7 +5305,7 @@ export class ClineProvider const postLoadBuffer = this.viewLocalState const mergedState: Partial = { ...loadedState } - if (!Object.is(preLoadBuffer.mode, postLoadBuffer.mode)) { + if (preLoadVersions.mode !== this.viewLocalStateWriteVersions.mode) { if (postLoadBuffer.mode === undefined) { delete mergedState.mode } else { @@ -5301,7 +5313,7 @@ export class ClineProvider } } - if (!Object.is(preLoadBuffer.currentApiConfigName, postLoadBuffer.currentApiConfigName)) { + if (preLoadVersions.currentApiConfigName !== this.viewLocalStateWriteVersions.currentApiConfigName) { if (postLoadBuffer.currentApiConfigName === undefined) { delete mergedState.currentApiConfigName } else { @@ -5309,7 +5321,7 @@ export class ClineProvider } } - if (!Object.is(preLoadBuffer.apiConfiguration, postLoadBuffer.apiConfiguration)) { + if (preLoadVersions.apiConfiguration !== this.viewLocalStateWriteVersions.apiConfiguration) { if (postLoadBuffer.apiConfiguration === undefined) { delete mergedState.apiConfiguration } else { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 83eea4a6ed..67b6e3df96 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3124,6 +3124,7 @@ 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", @@ -3616,6 +3617,7 @@ describe("ClineProvider", () => { // 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() }) From 9cb3635e033428ee74c1a63fc67ccb447fd41320 Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 13:13:56 +0800 Subject: [PATCH 8/9] chore: tighten the N1 test comments --- src/core/webview/__tests__/ClineProvider.spec.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 67b6e3df96..9ebf761f9c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3590,9 +3590,7 @@ describe("ClineProvider", () => { await provider["setViewStateId"]("__proto__") - // "__proto__" is rejected before assignment so a per-view entry can never be - // keyed through the Object.prototype setter: the temporary id stays active and - // nothing is persisted under the reserved name. + // "__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({}) @@ -3631,21 +3629,17 @@ describe("ClineProvider", () => { contextProxy, new WebviewFocusTracker(), ) - // Seed a pre-launch entry under the temporary id through the real write path, - // so the re-key has real work to do and records this instance as its author. + // 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 so a later launch retries registration and the load instead of the - // guard early-returning for an id that was never persisted. + // 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, and the - // pre-launch entry lands under the registered id. + // 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") From ff4ae389a1d61d27fe588e71a45363a6df447c6d Mon Sep 17 00:00:00 2001 From: easonLiangWorldedtech <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:12:48 +0800 Subject: [PATCH 9/9] fix(lint): drop the now-unused load snapshot and log spy --- src/core/webview/ClineProvider.ts | 1 - src/core/webview/__tests__/ClineProvider.spec.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 7e60740ebb..b1426f4ebe 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -5265,7 +5265,6 @@ export class ClineProvider // Snapshot the in-memory buffer before the async profile lookup. The // mutation paths update viewLocalState in place, so a shallow copy is // what makes fields mutated during the load window observable below. - const preLoadBuffer = { ...this.viewLocalState } const preLoadVersions = { ...this.viewLocalStateWriteVersions } if (persisted?.mode) { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 9ebf761f9c..350b4ffcd0 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3609,7 +3609,6 @@ describe("ClineProvider", () => { await provider.contextProxy.setValue("viewStates", { "stable-view": { claimToken: "host-token", mode: "architect", currentApiConfigName: "cfg-a" }, }) - const logSpy = vi.spyOn(provider, "log") await provider["setViewStateId"]("stable-view", "host-token")