From 76ab52f6889abff0b111078d2b651ba6ec6d53ec Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:45:20 +0800 Subject: [PATCH 1/3] unit 3a: view state machinery, registration, and the boolean webview post Part of the split plan issued on upstream issue 1992 for PR 2226. Content authority is PR 2226 head bc724b8df51f6d5585ffbac14ada09809d99a5f0, and every ported line is verbatim from that source at the same path, selected by donor new-side line range against unit 2's head cbf11148aea5a7390e88759be077e6fd1c035068. Ranges here: ClineProvider.ts new-side 77 to 1149, 2419 to 2478 and 2537 to 2542; spec new- side 15 to 5270 plus the two postMessageToWebview mock reversions at 9667 and 9771; webviewMessageHandler.ts new-side 705 to 715 and its spec hunks at 131 to 132 and 2809 to 2826; src/__tests__/extension.spec.ts new-side 377; api-start-new-task.spec.ts new-side 22. The 2419 to 2478 and 2537 to 2542 groups and the two mock reversions at 377, 22, 9667 and 9771 came out of units 4 and 5 and travel here because 3a is the unit that ships Promise: a mockResolvedValue(undefined) feeding that method, and the donor's rewritten diagnosticsRequest callback that returns it into a Promise at line 2252. main brought the call site, this unit brought the boolean return, so the adaptation travels with the return type and not into the merge. It is a spec row, and the mutation gate charges executable paths, not spec rows. --- .../src/__tests__/global-settings.test.ts | 43 ++ packages/types/src/__tests__/index.test.ts | 5 + packages/types/src/global-settings.ts | 12 + packages/types/src/vscode-extension-host.ts | 11 + src/__tests__/extension.spec.ts | 2 +- src/core/config/ContextProxy.ts | 24 +- src/core/config/ProviderSettingsManager.ts | 15 +- .../config/__tests__/ContextProxy.spec.ts | 19 + .../__tests__/ProviderSettingsManager.spec.ts | 7 +- .../config/__tests__/importExport.spec.ts | 49 ++ src/core/config/importExport.ts | 7 +- src/core/task/__tests__/Task.spec.ts | 1 + src/core/webview/ClineProvider.ts | 566 +++++++++++++++++- .../webview/__tests__/ClineProvider.spec.ts | 28 +- .../__tests__/webviewMessageHandler.spec.ts | 51 +- src/core/webview/webviewMessageHandler.ts | 24 + .../__tests__/api-configuration.spec.ts | 49 +- .../__tests__/api-start-new-task.spec.ts | 2 +- src/extension/api.ts | 11 +- .../language-models/copilotModelBroadcast.ts | 2 +- 20 files changed, 892 insertions(+), 36 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 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/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index c56d5291f6..90170d4a27 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -4523,6 +4523,7 @@ describe("Cline", () => { const { ts, text, partial } = message.clineMessage messageUpdates.push({ ts, text, partial }) } + return true }) const task = new Task({ diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 6cec3bdbe7..97b45179e8 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -160,6 +160,19 @@ 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. + */ + +/** + * 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`. + */ + /** * 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 +250,48 @@ 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 } + +/** + * 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. */ + +/** 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 +303,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 +476,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 = "nov-2026-v3.96.0-settings-costs-reliability" // v3.96.0 chat settings, provider costs, and reliability @@ -430,13 +559,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 +732,411 @@ 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. */ /** * Override EventEmitter's on method to match TaskProviderLike interface */ @@ -1698,8 +2236,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 +2263,7 @@ export class ClineProvider elapsedMs: Date.now() - startedAt, taskId, }) - return + return false } try { @@ -1733,6 +2277,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 +2288,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 +2354,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 { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 464889418e..c17d46d656 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -3229,8 +3229,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 +3316,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 +3332,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 +3341,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 +3367,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 +3377,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 +3397,7 @@ describe("ClineProvider", () => { expect(postMessageSpy).toHaveBeenCalledOnce() expect(statePostSettled).toBe(false) - releasePost() + releasePost(true) await statePost expect(statePostSettled).toBe(true) }) @@ -3423,7 +3424,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 +3451,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) @@ -5307,7 +5308,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", @@ -8164,7 +8164,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([]), } @@ -8268,7 +8268,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 98c306a4f3..29014b9e35 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -129,6 +129,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(), @@ -2832,6 +2834,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 @@ -3137,8 +3157,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 c5abbc0a12..e5e1c33f11 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 606bb7725d..de04787b17 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, TaskMessagesReadError } from "../core/task-persistence/taskMessages" import { getErrorCode } from "../core/task-persistence/readFileWithMissingRetry" @@ -635,9 +636,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 { From 735dc228a6d6032a1886fed636e35aceefe77336 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Wed, 11 Nov 2026 01:46:43 +0800 Subject: [PATCH 2/3] unit 3a: answer the CodeRabbit rows on comments, dispose behaviour, and delivery tests ClineProvider.ts 174: the two dangling JSDoc blocks now sit on the declarations they describe. The buffer and durable note sits above type PersistedViewState. The narrow-generic note sits above savePersistedViewState, where the sentence named saveViewState, a name no declaration in this file carries, and now names the queued write that path takes. ClineProvider.ts 283: the orphan note about the fields a view owns describes type ViewLocalStateField, and no declaration in this tree carries that name. git blame attributes the note to 76ab52f6889, so it is PR-carried, and it is deleted here as dead text to return with the unit that carries the declaration. ClineProvider.ts 516: dispose now bumps viewStateGeneration at its start and detaches this provider's entries from the shared write queue through the splice path the queue helper already provides, clearing entry.provider so a disposed view stops being retained behind another view's write. The comment at 503 and the block at 506 claimed both, and the code did neither. Two tests added, one that fails without the bump and one that fails without the detach. ClineProvider.ts 1045: three tests added for the survivors this chain owes itself, the shared view id counter, the per-field write version record, and the boolean delivery result reporting an undelivered message after dispose. webviewMessageHandler.spec.ts 2853: the ordering assertion is now causal. The row establishes isViewLaunched false, captures it inside the flush spy, and asserts the transition, because clearAllMocks resets call records and not properties. The rejected flush row the reviewer said was untested is added beside it, asserting the launch still completes. ProviderSettingsManager.ts 521 is not touched here. The guard is main content that predates this chain, and the file reaches this PR only through unit 2. --- src/core/webview/ClineProvider.ts | 26 ++++++--- .../webview/__tests__/ClineProvider.spec.ts | 58 +++++++++++++++++++ .../__tests__/webviewMessageHandler.spec.ts | 25 +++++++- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 97b45179e8..f94eb1fbec 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -160,18 +160,12 @@ 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. */ -/** - * 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 PersistedViewState = NonNullable[string] /** * https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts @@ -280,8 +274,6 @@ type ViewStateWriteEntry = { readonly run: (provider: ClineProvider) => Promise } -/** The fields a view owns in its buffer and in its durable record. */ - /** 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) { @@ -1489,6 +1481,22 @@ export class ClineProvider } this._disposed = true + // Detach this provider from the shared view-state write queue and retire its generation, + // so a closure that starts after this point drops its effects instead of applying them + // to a disposed provider and holding it retained behind another view's write. + this.viewStateGeneration++ + + 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 = [] this._postStateToWebviewThrottled.cancel() this.stopWebviewWatchdog() this.diagnosticsRecorder.record({ boundary: "provider", phase: "start", type: "dispose" }) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index c17d46d656..0be030752c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -2799,6 +2799,64 @@ describe("ClineProvider", () => { expect(mockWebviewPanel.webview.html).toBe(html) }) + test("retires the view-state generation when the provider disposes", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + const generation = provider["viewStateGeneration"] + + await provider.dispose() + + expect(provider["viewStateGeneration"]).toBe(generation + 1) + expect(provider["isViewStateGenerationLive"](generation)).toBe(false) + }) + + test("detaches this provider's queued view-state writes when it disposes", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + const entry: { + provider: ClineProvider | null + generation: number + run: (owner: ClineProvider) => Promise + } = { + provider, + generation: provider["viewStateGeneration"], + run: async () => {}, + } + // The pump advances on the fake clock, so the entry is read while it still waits on the queue. + const queued = ClineProvider["enqueuePersistedViewStateWrite"](entry) + + expect(ClineProvider["pendingViewStateWrites"]).toContain(entry) + expect(provider["scheduledViewStateWrites"]).toContain(entry) + + await provider.dispose() + + expect(ClineProvider["pendingViewStateWrites"]).not.toContain(entry) + expect(provider["scheduledViewStateWrites"]).toEqual([]) + expect(entry.provider).toBeNull() + + void queued + }) + + test("numbers this provider's view id from the shared counter", () => { + const suffix = Number(String(provider["viewId"]).split("-").pop()) + + expect(Number.isNaN(suffix)).toBe(false) + expect(ClineProvider["nextViewId"]).toBeGreaterThan(suffix) + }) + + test("keeps the per-field view-state write versions the record gates on", () => { + expect(provider["viewLocalStateWriteVersions"]).toEqual({ + mode: 0, + currentApiConfigName: 0, + apiConfiguration: 0, + }) + }) + + test("reports a message as undelivered once the provider is disposed", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + await provider.dispose() + + expect(await provider.postMessageToWebview({ type: "mcpServers", mcpServers: [] })).toBe(false) + }) + test.each([false, true])( "replaces the tab host without disposing its provider or task (interrupted: %s)", async (interrupted) => { diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 29014b9e35..0ea700671b 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -2841,15 +2841,34 @@ describe("webviewMessageHandler - telemetrySetting", () => { // 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 + // Establish the false value inside this row: every beforeEach in this file runs + // clearAllMocks, which resets call records and not properties, so a value an earlier row + // left set could make the ordering pass on its own. + mockClineProvider.isViewLaunched = false + const launchedWhenFlushed: boolean[] = [] vi.mocked(mockClineProvider.deliverPendingViewStateClaims).mockImplementationOnce(async () => { - launchedWhenFlushed = mockClineProvider.isViewLaunched + launchedWhenFlushed.push(mockClineProvider.isViewLaunched) }) await webviewMessageHandler(mockClineProvider, { type: "webviewDidLaunch" }) expect(mockClineProvider.deliverPendingViewStateClaims).toHaveBeenCalledTimes(1) - expect(launchedWhenFlushed).toBe(true) + expect(launchedWhenFlushed).toEqual([true]) + }) + + it("contains a rejected claim flush so webviewDidLaunch still completes", async () => { + vi.mocked(mockClineProvider.customModesManager.getCustomModes).mockResolvedValue([]) + vi.mocked(mockClineProvider.deliverPendingViewStateClaims).mockImplementationOnce(() => { + throw new Error("Claim flush failed") + }) + + await expect(webviewMessageHandler(mockClineProvider, { type: "webviewDidLaunch" })).resolves.toBeUndefined() + + expect(mockClineProvider.deliverPendingViewStateClaims).toHaveBeenCalledTimes(1) + + expect(mockClineProvider.log).toHaveBeenCalledWith( + expect.stringContaining("Flushing pending claims on launch failed"), + ) }) // CodeRabbit follow-up on the finding #12 fix: webviewDidLaunch's telemetry init read state From ca2dc281f6d6e3f8e7023b40b540a394dd225c35 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Wed, 11 Nov 2026 02:22:34 +0800 Subject: [PATCH 3/3] unit 3a: assert the detach loop's splice branch in dispose The detach loop added to dispose() carries five mutants the digest at this head lists as Survived: four on the splice guard (ConditionalExpression on both arms, the EqualityOperator on the index test, and the UnaryOperator on the delete count) and one CallExpression that removes the splice itself. Two rows discriminate them: one where the entry has already left the shared queue, so the guard must not splice and a foreign entry sitting on the queue is still there afterwards, and one where the entry is still queued, so the splice runs, the entry is gone, and the entry behind it is untouched. Both rows assert that the scheduled entry's provider field is cleared, which is what releases the provider. The not-found shape is the one the queue step itself produces: it removes the entry from the shared list at its own turn before it asks whether the owner's generation is still live, so a write whose owner went stale stays on that provider's schedule while it is gone from the list. Verification: eslint on the file exits 0 with --max-warnings=0 and the file's suppression entry stays at 195 no-explicit-any; the focused spec runs 349 tests with 349 passing, up from 347. --- .../webview/__tests__/ClineProvider.spec.ts | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 0be030752c..338c52800d 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -2835,6 +2835,98 @@ describe("ClineProvider", () => { void queued }) + test("leaves the shared queue untouched when dispose finds the entry already drained", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + const drained: { + provider: ClineProvider | null + generation: number + run: (owner: ClineProvider) => Promise + } = { + provider, + generation: provider["viewStateGeneration"], + run: async () => {}, + } + const queuedDrained = ClineProvider["enqueuePersistedViewStateWrite"](drained) + // The queue step drops the entry from the shared list at its own turn before it asks + // whether the owner's generation is still live, so a write whose owner went stale + // stays on this provider's schedule while it is already gone from the shared list. + // That is the state below, built the way the queue step builds it. + const drainedIndex = ClineProvider["pendingViewStateWrites"].indexOf(drained) + ClineProvider["pendingViewStateWrites"].splice(drainedIndex, 1) + // A write belonging to somebody else, left on the shared queue so that a splice + // run without a match would remove it. + const neighbour: { + provider: ClineProvider | null + generation: number + run: (owner: ClineProvider) => Promise + } = { + provider: null, + generation: 0, + run: async () => {}, + } + ClineProvider["pendingViewStateWrites"].push(neighbour) + const sharedBefore = ClineProvider["pendingViewStateWrites"].slice() + + await provider.dispose() + + expect(ClineProvider["pendingViewStateWrites"]).toEqual(sharedBefore) + expect(ClineProvider["pendingViewStateWrites"]).toContain(neighbour) + expect(provider["scheduledViewStateWrites"]).toEqual([]) + expect(drained.provider).toBeNull() + expect(ClineProvider["pendingViewStateWrites"]).toHaveLength(sharedBefore.length) + + // The neighbour belongs to no provider, so this dispose leaves it on the shared + // queue; take it back out rather than hand it to the next test. + const neighbourIndex = ClineProvider["pendingViewStateWrites"].indexOf(neighbour) + if (neighbourIndex !== -1) { + ClineProvider["pendingViewStateWrites"].splice(neighbourIndex, 1) + } + + void queuedDrained + }) + + test("splices the provider entry out of the shared queue without touching the entry behind it", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + const entry: { + provider: ClineProvider | null + generation: number + run: (owner: ClineProvider) => Promise + } = { + provider, + generation: provider["viewStateGeneration"], + run: async () => {}, + } + const queued = ClineProvider["enqueuePersistedViewStateWrite"](entry) + // A write from another provider that is already waiting behind this one. + const behind: { + provider: ClineProvider | null + generation: number + run: (owner: ClineProvider) => Promise + } = { + provider: null, + generation: 0, + run: async () => {}, + } + ClineProvider["pendingViewStateWrites"].push(behind) + + const entryIndex = ClineProvider["pendingViewStateWrites"].indexOf(entry) + expect(ClineProvider["pendingViewStateWrites"].indexOf(behind)).toBe(entryIndex + 1) + + await provider.dispose() + + expect(ClineProvider["pendingViewStateWrites"]).not.toContain(entry) + expect(ClineProvider["pendingViewStateWrites"]).toContain(behind) + expect(provider["scheduledViewStateWrites"]).toEqual([]) + expect(entry.provider).toBeNull() + + const behindIndex = ClineProvider["pendingViewStateWrites"].indexOf(behind) + if (behindIndex !== -1) { + ClineProvider["pendingViewStateWrites"].splice(behindIndex, 1) + } + + void queued + }) + test("numbers this provider's view id from the shared counter", () => { const suffix = Number(String(provider["viewId"]).split("-").pop())