From 19ebb09fc6afc1d4954237b09e0aaf45d7bdee1b Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sun, 20 Sep 2026 22:17:06 +0900 Subject: [PATCH 01/18] fix(webview): detect dead webview renderer via heartbeat and auto-reload --- packages/types/src/vscode-extension-host.ts | 2 + src/core/webview/ClineProvider.ts | 50 +++++++++++ .../webview/__tests__/ClineProvider.spec.ts | 90 +++++++++++++++++++ src/core/webview/webviewMessageHandler.ts | 4 + webview-ui/src/App.tsx | 9 ++ webview-ui/src/__tests__/App.spec.tsx | 38 ++++++++ 6 files changed, 193 insertions(+) diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 5f6b579779..84e03d0980 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -472,6 +472,7 @@ export interface WebviewMessage { | "getListApiConfiguration" | "customInstructions" | "webviewDidLaunch" + | "webviewHeartbeat" | "newTask" | "askResponse" | "terminalOperation" @@ -696,6 +697,7 @@ export interface WebviewMessage { ids?: string[] terminalOperation?: "continue" | "abort" messageTs?: number + timestamp?: number // For webviewHeartbeat restoreCheckpoint?: boolean historyPreviewCollapsed?: boolean filters?: { type?: string; search?: string; tags?: string[] } diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 86ce5d8e67..64c64f7dc5 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -223,6 +223,10 @@ export class ClineProvider private taskEventListeners: WeakMap void>> = new WeakMap() private currentWorkspacePath: string | undefined private _disposed = false + private lastWebviewHeartbeatAt = 0 + private webviewWatchdogInterval: ReturnType | null = null + private static readonly WEBVIEW_WATCHDOG_TICK_MS = 60_000 + private static readonly WEBVIEW_HEARTBEAT_STALE_MS = 90_000 private readonly _postStateToWebviewThrottled = debounce( async () => { try { @@ -842,6 +846,10 @@ export class ClineProvider this._disposed = true this._postStateToWebviewThrottled.cancel() + if (this.webviewWatchdogInterval) { + clearInterval(this.webviewWatchdogInterval) + this.webviewWatchdogInterval = null + } this.log("Disposing ClineProvider...") // Reject any tasks still waiting for a scheduler permit so they don't @@ -1082,6 +1090,9 @@ export class ClineProvider // and executes code based on the message that is received. this.setWebviewMessageListener(webviewView.webview) + // Detect a dead webview renderer process (gray screen) via heartbeat timeout. + this.startWebviewWatchdog() + // Initialize code index status subscription for the current workspace. this.updateCodeIndexStatusSubscription() @@ -1100,6 +1111,9 @@ export class ClineProvider // for this visibility listener panel. const viewStateDisposable = webviewView.onDidChangeViewState(() => { if (this.view?.visible) { + // Hidden webviews throttle timers, so grant a fresh grace + // window instead of counting throttled heartbeats as a crash. + this.updateWebviewHeartbeat() void this.postMessageToWebview({ type: "action", action: "didBecomeVisible" }) } else { this.logWebviewHiddenDiagnostics() @@ -1111,6 +1125,9 @@ export class ClineProvider // sidebar const visibilityDisposable = webviewView.onDidChangeVisibility(() => { if (this.view?.visible) { + // Hidden webviews throttle timers, so grant a fresh grace + // window instead of counting throttled heartbeats as a crash. + this.updateWebviewHeartbeat() void this.postMessageToWebview({ type: "action", action: "didBecomeVisible" }) } else { this.logWebviewHiddenDiagnostics() @@ -3381,6 +3398,39 @@ export class ClineProvider ) } + /** Records that the webview renderer is alive; called on every webviewHeartbeat message. */ + public updateWebviewHeartbeat(): void { + this.lastWebviewHeartbeatAt = Date.now() + } + + /** + * Starts (or restarts) the watchdog that detects a dead webview renderer + * process. Hidden webviews throttle timers, so becoming visible resets the + * grace window instead of counting throttled heartbeats as a crash. + */ + private startWebviewWatchdog(): void { + this.updateWebviewHeartbeat() + if (this.webviewWatchdogInterval) { + clearInterval(this.webviewWatchdogInterval) + } + this.webviewWatchdogInterval = setInterval(() => { + if (this.view?.visible !== true) { + return + } + if (Date.now() - this.lastWebviewHeartbeatAt <= ClineProvider.WEBVIEW_HEARTBEAT_STALE_MS) { + return + } + this.log("[Zoo Code] Webview heartbeat stale while visible; reloading webview (dead renderer?)") + void Promise.resolve(vscode.commands.executeCommand("workbench.action.webview.reloadWebviewAction")).catch( + (error) => { + this.log( + `[Zoo Code] Failed to reload webview: ${error instanceof Error ? error.message : String(error)}`, + ) + }, + ) + }, ClineProvider.WEBVIEW_WATCHDOG_TICK_MS) + } + public getRecentTasks(): string[] { if (this.recentTasksCache) { return this.recentTasksCache diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 97c4dd877e..dfd165f678 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -663,6 +663,96 @@ describe("ClineProvider", () => { }) }) + describe("webview heartbeat watchdog", () => { + let visibilityCallback: () => void + + beforeEach(() => { + // Fake timers must be active before resolveWebviewView so the + // watchdog interval is registered on the fake clock. + vi.useFakeTimers() + mockWebviewView.onDidChangeVisibility = vi.fn().mockImplementation((cb: () => void) => { + visibilityCallback = cb + return { dispose: vi.fn() } + }) + }) + + afterEach(async () => { + await provider.dispose() + vi.useRealTimers() + }) + + test("does not reload the webview while heartbeats are fresh", async () => { + await provider.resolveWebviewView(mockWebviewView) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await vi.advanceTimersByTimeAsync(110_000) + await webviewMessageHandler(provider, { type: "webviewHeartbeat", timestamp: Date.now() }) + await vi.advanceTimersByTimeAsync(60_000) + + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + }) + + test("reloads the webview when the heartbeat is stale and the view is visible", async () => { + await provider.resolveWebviewView(mockWebviewView) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await vi.advanceTimersByTimeAsync(120_000) + + expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.webview.reloadWebviewAction") + }) + + test("does not reload while the view is hidden even when the heartbeat is stale", async () => { + await provider.resolveWebviewView(mockWebviewView) + Object.defineProperty(mockWebviewView, "visible", { value: false, configurable: true }) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await vi.advanceTimersByTimeAsync(180_000) + + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + }) + + test("resets the grace window when the view becomes visible", async () => { + await provider.resolveWebviewView(mockWebviewView) + Object.defineProperty(mockWebviewView, "visible", { value: false, configurable: true }) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await vi.advanceTimersByTimeAsync(120_000) + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + + Object.defineProperty(mockWebviewView, "visible", { value: true, configurable: true }) + visibilityCallback() + + await vi.advanceTimersByTimeAsync(60_000) + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + + // Watchdog ticks every 60s; 120s after the flip the heartbeat is stale again. + await vi.advanceTimersByTimeAsync(60_000) + expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) + expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.webview.reloadWebviewAction") + }) + + test("stops watching after the provider is disposed", async () => { + await provider.resolveWebviewView(mockWebviewView) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await provider.dispose() + await vi.advanceTimersByTimeAsync(180_000) + + expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + }) + + test("does not stack watchdog intervals when resolveWebviewView runs again", async () => { + await provider.resolveWebviewView(mockWebviewView) + await provider.resolveWebviewView(mockWebviewView) + vi.mocked(vscode.commands.executeCommand).mockClear() + + await vi.advanceTimersByTimeAsync(120_000) + + expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) + }) + }) + test("resolveWebviewView sets up webview correctly in development mode even if local server is not running", async () => { provider = new ClineProvider( { ...mockContext, extensionMode: vscode.ExtensionMode.Development }, diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 34a35ea3ca..d5408cca0b 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -688,6 +688,10 @@ export const webviewMessageHandler = async ( provider.isViewLaunched = true break + case "webviewHeartbeat": + // Timestamp-only update for the dead-renderer watchdog; no other side effects. + provider.updateWebviewHeartbeat() + break case "newTask": // Initializing new instance of Cline will make sure that any // agentically running promises in old instance don't affect our new diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index b1fbf82999..f4e80ac480 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -206,6 +206,15 @@ const App = () => { // Tell the extension that we are ready to receive messages. useEffect(() => vscode.postMessage({ type: "webviewDidLaunch" }), []) + // Heartbeat so the extension watchdog can detect a crashed webview renderer + // process (gray screen) and reload the view. + useEffect(() => { + const postHeartbeat = () => vscode.postMessage({ type: "webviewHeartbeat", timestamp: Date.now() }) + postHeartbeat() + const interval = setInterval(postHeartbeat, 30_000) + return () => clearInterval(interval) + }, []) + // Initialize source map support for better error reporting useEffect(() => { // Initialize source maps for better error reporting in production diff --git a/webview-ui/src/__tests__/App.spec.tsx b/webview-ui/src/__tests__/App.spec.tsx index 137bed5d70..4178b999e6 100644 --- a/webview-ui/src/__tests__/App.spec.tsx +++ b/webview-ui/src/__tests__/App.spec.tsx @@ -4,6 +4,7 @@ import React from "react" import { render, screen, act, cleanup } from "@/utils/test-utils" import AppWithProviders from "../App" +import { vscode } from "@src/utils/vscode" vi.mock("@src/utils/vscode", () => ({ vscode: { @@ -476,4 +477,41 @@ describe("App", () => { expect(chatView.getAttribute("data-hidden")).toBe("false") expect(screen.queryByTestId("marketplace-view")).not.toBeInTheDocument() }) + + describe("webview heartbeat", () => { + afterEach(() => { + vi.useRealTimers() + }) + + it("posts an immediate heartbeat on mount and then every 30 seconds", () => { + vi.useFakeTimers() + render() + + const postMessageMock = vi.mocked(vscode.postMessage) + expect(postMessageMock).toHaveBeenCalledWith( + expect.objectContaining({ type: "webviewHeartbeat", timestamp: expect.any(Number) }), + ) + const callsAfterMount = postMessageMock.mock.calls.length + + vi.advanceTimersByTime(30_000) + expect(postMessageMock.mock.calls.length).toBe(callsAfterMount + 1) + + vi.advanceTimersByTime(30_000) + expect(postMessageMock.mock.calls.length).toBe(callsAfterMount + 2) + expect(postMessageMock).toHaveBeenLastCalledWith(expect.objectContaining({ type: "webviewHeartbeat" })) + }) + + it("stops posting heartbeats after unmount", () => { + vi.useFakeTimers() + const { unmount } = render() + + const postMessageMock = vi.mocked(vscode.postMessage) + const callsAfterMount = postMessageMock.mock.calls.length + + unmount() + vi.advanceTimersByTime(90_000) + + expect(postMessageMock.mock.calls.length).toBe(callsAfterMount) + }) + }) }) From 77d02f3f85add531443765e5e2b580e868f0615a Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sun, 20 Sep 2026 23:28:36 +0900 Subject: [PATCH 02/18] fix(webview): scope recovery reload to the provider's own webview --- src/core/webview/ClineProvider.ts | 39 +++++--- .../webview/__tests__/ClineProvider.spec.ts | 94 +++++++++++++++---- 2 files changed, 104 insertions(+), 29 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 64c64f7dc5..620a24cc8a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1050,11 +1050,7 @@ export class ClineProvider localResourceRoots: resourceRoots, } - webviewView.webview.html = - this.contextProxy.extensionMode === vscode.ExtensionMode.Development && - process.env.ROO_CODE_THEME_FIXTURE_PROBE !== "1" - ? await this.getHMRHtmlContent(webviewView.webview) - : await this.getHtmlContent(webviewView.webview) + webviewView.webview.html = await this.getWebviewHtml(webviewView.webview) // Initialize out-of-scope variables that need to receive persistent // global state values. @@ -3421,16 +3417,35 @@ export class ClineProvider return } this.log("[Zoo Code] Webview heartbeat stale while visible; reloading webview (dead renderer?)") - void Promise.resolve(vscode.commands.executeCommand("workbench.action.webview.reloadWebviewAction")).catch( - (error) => { - this.log( - `[Zoo Code] Failed to reload webview: ${error instanceof Error ? error.message : String(error)}`, - ) - }, - ) + void this.reloadWebviewForRecovery() }, ClineProvider.WEBVIEW_WATCHDOG_TICK_MS) } + /** + * Reloads only this provider's own webview by regenerating its HTML (fresh + * nonce) and reassigning `webview.html`, which forces VS Code to reload that + * webview. Works for both sidebar (WebviewView) and tab (WebviewPanel) shapes. + */ + private async reloadWebviewForRecovery(): Promise { + const view = this.view + if (!view?.webview) { + return + } + try { + view.webview.html = await this.getWebviewHtml(view.webview) + } catch (error) { + this.log(`[Zoo Code] Failed to reload webview: ${error instanceof Error ? error.message : String(error)}`) + } + } + + /** Builds the webview HTML using the same path as resolveWebviewView (HMR in development). */ + private async getWebviewHtml(webview: vscode.Webview): Promise { + return this.contextProxy.extensionMode === vscode.ExtensionMode.Development && + process.env.ROO_CODE_THEME_FIXTURE_PROBE !== "1" + ? await this.getHMRHtmlContent(webview) + : await this.getHtmlContent(webview) + } + public getRecentTasks(): string[] { if (this.recentTasksCache) { return this.recentTasksCache diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index dfd165f678..b5e6277576 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -683,73 +683,133 @@ describe("ClineProvider", () => { test("does not reload the webview while heartbeats are fresh", async () => { await provider.resolveWebviewView(mockWebviewView) - vi.mocked(vscode.commands.executeCommand).mockClear() + const htmlAfterResolve = mockWebviewView.webview.html await vi.advanceTimersByTimeAsync(110_000) await webviewMessageHandler(provider, { type: "webviewHeartbeat", timestamp: Date.now() }) await vi.advanceTimersByTimeAsync(60_000) - expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) - test("reloads the webview when the heartbeat is stale and the view is visible", async () => { + test("reloads the webview with fresh HTML when the heartbeat is stale and the view is visible", async () => { await provider.resolveWebviewView(mockWebviewView) - vi.mocked(vscode.commands.executeCommand).mockClear() + const htmlAfterResolve = mockWebviewView.webview.html await vi.advanceTimersByTimeAsync(120_000) - expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) - expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.webview.reloadWebviewAction") + // Reassigning webview.html with a fresh nonce is what forces the reload. + expect(mockWebviewView.webview.html).not.toBe(htmlAfterResolve) + expect(mockWebviewView.webview.html).toContain("Zoo Code") }) test("does not reload while the view is hidden even when the heartbeat is stale", async () => { await provider.resolveWebviewView(mockWebviewView) Object.defineProperty(mockWebviewView, "visible", { value: false, configurable: true }) - vi.mocked(vscode.commands.executeCommand).mockClear() + const htmlAfterResolve = mockWebviewView.webview.html await vi.advanceTimersByTimeAsync(180_000) - expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) test("resets the grace window when the view becomes visible", async () => { await provider.resolveWebviewView(mockWebviewView) Object.defineProperty(mockWebviewView, "visible", { value: false, configurable: true }) - vi.mocked(vscode.commands.executeCommand).mockClear() + const htmlAfterResolve = mockWebviewView.webview.html await vi.advanceTimersByTimeAsync(120_000) - expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) Object.defineProperty(mockWebviewView, "visible", { value: true, configurable: true }) visibilityCallback() + const htmlAfterBecomingVisible = mockWebviewView.webview.html await vi.advanceTimersByTimeAsync(60_000) - expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + expect(mockWebviewView.webview.html).toBe(htmlAfterBecomingVisible) // Watchdog ticks every 60s; 120s after the flip the heartbeat is stale again. await vi.advanceTimersByTimeAsync(60_000) - expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) - expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.webview.reloadWebviewAction") + expect(mockWebviewView.webview.html).not.toBe(htmlAfterBecomingVisible) }) test("stops watching after the provider is disposed", async () => { await provider.resolveWebviewView(mockWebviewView) - vi.mocked(vscode.commands.executeCommand).mockClear() + const htmlAfterResolve = mockWebviewView.webview.html await provider.dispose() await vi.advanceTimersByTimeAsync(180_000) - expect(vscode.commands.executeCommand).not.toHaveBeenCalled() + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) test("does not stack watchdog intervals when resolveWebviewView runs again", async () => { await provider.resolveWebviewView(mockWebviewView) await provider.resolveWebviewView(mockWebviewView) - vi.mocked(vscode.commands.executeCommand).mockClear() + + // getHtmlContent regenerates the HTML via one getState() call per reload, + // so getState invocations after this point count watchdog reloads. + const getStateSpy = vi.spyOn(provider, "getState") + + await vi.advanceTimersByTimeAsync(120_000) + + expect(getStateSpy).toHaveBeenCalledTimes(1) + }) + + test("logs when regenerating the reload HTML fails", async () => { + await provider.resolveWebviewView(mockWebviewView) + vi.spyOn(provider, "getState").mockRejectedValue(new Error("regen boom")) + ;(mockOutputChannel.appendLine as ReturnType).mockClear() await vi.advanceTimersByTimeAsync(120_000) - expect(vscode.commands.executeCommand).toHaveBeenCalledTimes(1) + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("[Zoo Code] Failed to reload webview: regen boom"), + ) + }) + + test("reloads only its own webview when multiple providers are active", async () => { + // Structural stand-in for the VS Code webview API surface this scenario + // exercises, same as the mockContext cast below. + const mockWebviewViewB = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn().mockImplementation((callback: () => void) => { + callback() + return { dispose: vi.fn() } + }), + onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + } as unknown as vscode.WebviewView + const providerB = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + ) + try { + await provider.resolveWebviewView(mockWebviewView) + await providerB.resolveWebviewView(mockWebviewViewB) + + const htmlBeforeA = mockWebviewView.webview.html + const htmlBeforeB = mockWebviewViewB.webview.html + + await vi.advanceTimersByTimeAsync(119_000) + // Keep provider B's heartbeat fresh so only A's watchdog fires. + await webviewMessageHandler(providerB, { type: "webviewHeartbeat", timestamp: Date.now() }) + await vi.advanceTimersByTimeAsync(1_000) + + expect(mockWebviewView.webview.html).not.toBe(htmlBeforeA) + expect(mockWebviewViewB.webview.html).toBe(htmlBeforeB) + } finally { + await providerB.dispose() + } }) }) From dfbcc6ee00d656b499cdda93cdfbcaba90978a53 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 04:26:21 +0900 Subject: [PATCH 03/18] fix(webview): stop recovery watchdog when sidebar webview is disposed --- src/core/webview/ClineProvider.ts | 19 ++++++-- .../webview/__tests__/ClineProvider.spec.ts | 45 ++++++++++++------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 620a24cc8a..25d4bfb296 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -820,6 +820,7 @@ export class ClineProvider */ private clearWebviewResources() { this.rejectPendingThemeFixtureProbes(new Error("Webview was disposed before the theme fixture probe completed")) + this.stopWebviewWatchdog() while (this.webviewDisposables.length) { const x = this.webviewDisposables.pop() if (x) { @@ -846,10 +847,7 @@ export class ClineProvider this._disposed = true this._postStateToWebviewThrottled.cancel() - if (this.webviewWatchdogInterval) { - clearInterval(this.webviewWatchdogInterval) - this.webviewWatchdogInterval = null - } + this.stopWebviewWatchdog() this.log("Disposing ClineProvider...") // Reject any tasks still waiting for a scheduler permit so they don't @@ -1143,6 +1141,11 @@ export class ClineProvider } else { this.log("Clearing webview resources for sidebar view") this.clearWebviewResources() + if (this.view === webviewView) { + // Drop the disposed view so nothing keeps polling it + // (e.g. the recovery watchdog) for the provider's lifetime. + this.view = undefined + } // Reset current workspace manager reference when view is disposed this.codeIndexManager = undefined } @@ -3421,6 +3424,14 @@ export class ClineProvider }, ClineProvider.WEBVIEW_WATCHDOG_TICK_MS) } + /** Stops the renderer heartbeat watchdog; the webview it watches is gone. */ + private stopWebviewWatchdog(): void { + if (this.webviewWatchdogInterval) { + clearInterval(this.webviewWatchdogInterval) + this.webviewWatchdogInterval = null + } + } + /** * Reloads only this provider's own webview by regenerating its HTML (fresh * nonce) and reassigning `webview.html`, which forces VS Code to reload that diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index b5e6277576..420cfd904e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -527,10 +527,7 @@ describe("ClineProvider", () => { cspSource: "vscode-webview://test-csp-source", }, visible: true, - onDidDispose: vi.fn().mockImplementation((callback) => { - callback() - return { dispose: vi.fn() } - }), + onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn().mockImplementation(() => { return { dispose: vi.fn() } }), @@ -665,6 +662,7 @@ describe("ClineProvider", () => { describe("webview heartbeat watchdog", () => { let visibilityCallback: () => void + let disposeCallback: () => void beforeEach(() => { // Fake timers must be active before resolveWebviewView so the @@ -674,6 +672,10 @@ describe("ClineProvider", () => { visibilityCallback = cb return { dispose: vi.fn() } }) + mockWebviewView.onDidDispose = vi.fn().mockImplementation((cb: () => void) => { + disposeCallback = cb + return { dispose: vi.fn() } + }) }) afterEach(async () => { @@ -743,6 +745,23 @@ describe("ClineProvider", () => { expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) + test("stops watching and clears the view when the sidebar webview is disposed", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + // The provider outlives a disposed sidebar view; VS Code re-resolves + // a fresh view later. Disposal must stop the watchdog and drop the + // stale view reference so no recovery reload targets the dead view. + disposeCallback() + expect(provider["webviewWatchdogInterval"]).toBeNull() + // @ts-ignore - accessing private property for testing + expect(provider.view).toBeUndefined() + + await vi.advanceTimersByTimeAsync(180_000) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + test("does not stack watchdog intervals when resolveWebviewView runs again", async () => { await provider.resolveWebviewView(mockWebviewView) await provider.resolveWebviewView(mockWebviewView) @@ -781,10 +800,7 @@ describe("ClineProvider", () => { cspSource: "vscode-webview://test-csp-source", }, visible: true, - onDidDispose: vi.fn().mockImplementation((callback: () => void) => { - callback() - return { dispose: vi.fn() } - }), + onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), } as unknown as vscode.WebviewView const providerB = new ClineProvider( @@ -801,7 +817,8 @@ describe("ClineProvider", () => { const htmlBeforeB = mockWebviewViewB.webview.html await vi.advanceTimersByTimeAsync(119_000) - // Keep provider B's heartbeat fresh so only A's watchdog fires. + // B's view is still alive; refresh its heartbeat so a stale one + // would reload it too, leaving only A's watchdog to fire. await webviewMessageHandler(providerB, { type: "webviewHeartbeat", timestamp: Date.now() }) await vi.advanceTimersByTimeAsync(1_000) @@ -3867,10 +3884,7 @@ describe("ClineProvider - Router Models", () => { asWebviewUri: vi.fn(), }, visible: true, - onDidDispose: vi.fn().mockImplementation((callback) => { - callback() - return { dispose: vi.fn() } - }), + onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn().mockImplementation(() => { return { dispose: vi.fn() } }), @@ -4221,10 +4235,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { asWebviewUri: vi.fn(), }, visible: true, - onDidDispose: vi.fn().mockImplementation((callback) => { - callback() - return { dispose: vi.fn() } - }), + onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn().mockImplementation(() => { return { dispose: vi.fn() } }), From 36e6bcb757d30aa116d8456e245f38521c1ce333 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Mon, 21 Sep 2026 05:35:12 +0900 Subject: [PATCH 04/18] test(webview): capture sidebar dispose callback in task history spec --- src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 2bbf0736c6..8b4cb5575f 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -339,10 +339,7 @@ describe("ClineProvider Task History Synchronization", () => { cspSource: "vscode-webview://test-csp-source", }, visible: true, - onDidDispose: vi.fn().mockImplementation((callback) => { - callback() - return { dispose: vi.fn() } - }), + onDidDispose: vi.fn(), onDidChangeVisibility: vi.fn().mockImplementation(() => { return { dispose: vi.fn() } }), From ec7d3751e0e17ab13c80e4b30ab87b262b1cf188 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Wed, 23 Sep 2026 06:35:55 +0900 Subject: [PATCH 05/18] fix(webview): invalidate in-flight recovery reload on dispose or replace The watchdog fires reloadWebviewForRecovery() without tracking it, so the recovery could reassign webview.html after the provider was disposed or the watched view was disposed/replaced while the HTML was being generated. Track the operation with an epoch token: capture it before the await and bail out before assigning webview.html when the provider is disposed, the epoch changed, or this.view no longer references the captured view. clearWebviewResources() bumps the epoch so sidebar view disposal and provider disposal invalidate any in-flight recovery. Also add fake-timer watchdog coverage for the tab-panel branch (WebviewPanel shape): a hidden tab does not reload, the onDidChangeViewState callback resets the heartbeat grace window when the tab becomes visible, and a stale heartbeat reloads a visible tab. --- src/core/webview/ClineProvider.ts | 19 ++- .../webview/__tests__/ClineProvider.spec.ts | 153 ++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 25d4bfb296..4be16135c2 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -225,6 +225,10 @@ export class ClineProvider private _disposed = false private lastWebviewHeartbeatAt = 0 private webviewWatchdogInterval: ReturnType | null = null + // Bumped to invalidate an in-flight recovery reload when the provider or + // the watched view is disposed; the reload must not reassign webview.html + // afterwards. + private webviewRecoveryEpoch = 0 private static readonly WEBVIEW_WATCHDOG_TICK_MS = 60_000 private static readonly WEBVIEW_HEARTBEAT_STALE_MS = 90_000 private readonly _postStateToWebviewThrottled = debounce( @@ -821,6 +825,9 @@ export class ClineProvider private clearWebviewResources() { this.rejectPendingThemeFixtureProbes(new Error("Webview was disposed before the theme fixture probe completed")) this.stopWebviewWatchdog() + // Invalidate any recovery reload still awaiting its HTML so it cannot + // reassign webview.html on the disposed view. + this.webviewRecoveryEpoch++ while (this.webviewDisposables.length) { const x = this.webviewDisposables.pop() if (x) { @@ -3442,8 +3449,18 @@ export class ClineProvider if (!view?.webview) { return } + // Capture the epoch so a disposal or view replacement can invalidate + // this operation while the HTML is being generated. + const epoch = this.webviewRecoveryEpoch try { - view.webview.html = await this.getWebviewHtml(view.webview) + const html = await this.getWebviewHtml(view.webview) + // The await yields; assigning html now that the provider is disposed + // or the watched view was disposed/replaced would touch a dead or + // unrelated webview. + if (this._disposed || this.webviewRecoveryEpoch !== epoch || this.view !== view) { + return + } + view.webview.html = html } catch (error) { this.log(`[Zoo Code] Failed to reload webview: ${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 420cfd904e..88cf2977e2 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -787,6 +787,89 @@ describe("ClineProvider", () => { ) }) + test("does not reassign html when the provider is disposed mid-recovery", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + // Hold the recovery reload's HTML regeneration in flight until the + // disposal below lands. + let finishReload: (state: ExtensionState) => void = () => {} + vi.spyOn(provider, "getState").mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + // The stale heartbeat started a recovery reload, but its HTML is still pending. + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + await provider.dispose() + finishReload({ apiConfiguration: {} } as unknown as ExtensionState) + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + + test("does not reassign html when the sidebar view is disposed mid-recovery", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + let finishReload: (state: ExtensionState) => void = () => {} + vi.spyOn(provider, "getState").mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + disposeCallback() + finishReload({ apiConfiguration: {} } as unknown as ExtensionState) + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + + test("does not reassign html when the watched view is replaced mid-recovery", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + let finishReload: (state: ExtensionState) => void = () => {} + vi.spyOn(provider, "getState").mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + // VS Code re-resolves a fresh view (e.g. sidebar re-opened) while the + // recovery reload for the old view is still awaiting its HTML. + // @ts-ignore - accessing private property for testing + provider.view = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + } + + finishReload({ apiConfiguration: {} } as unknown as ExtensionState) + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + test("reloads only its own webview when multiple providers are active", async () => { // Structural stand-in for the VS Code webview API surface this scenario // exercises, same as the mockContext cast below. @@ -828,6 +911,76 @@ describe("ClineProvider", () => { await providerB.dispose() } }) + + describe("tab panel (WebviewPanel shape)", () => { + let viewStateCallback: () => void + // Structural stand-in for the VS Code webview API surface this + // scenario exercises, same as the mockWebviewViewB cast below. + let mockWebviewPanel: vscode.WebviewPanel + + beforeEach(() => { + // WebviewPanel-shaped stand-in: same webview surface, but the + // visibility listener is onDidChangeViewState instead of + // onDidChangeVisibility, matching resolveWebviewView's tab branch. + mockWebviewPanel = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + onDidChangeViewState: vi.fn().mockImplementation((cb: () => void) => { + viewStateCallback = cb + return { dispose: vi.fn() } + }), + dispose: vi.fn(), + } as unknown as vscode.WebviewPanel + }) + + test("reloads the tab webview when the heartbeat is stale and the tab is visible", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + const htmlAfterResolve = mockWebviewPanel.webview.html + + await vi.advanceTimersByTimeAsync(120_000) + + expect(mockWebviewPanel.webview.html).not.toBe(htmlAfterResolve) + expect(mockWebviewPanel.webview.html).toContain("Zoo Code") + }) + + test("does not reload a hidden tab even when the heartbeat is stale", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + Object.defineProperty(mockWebviewPanel, "visible", { value: false, configurable: true }) + const htmlAfterResolve = mockWebviewPanel.webview.html + + await vi.advanceTimersByTimeAsync(180_000) + + expect(mockWebviewPanel.webview.html).toBe(htmlAfterResolve) + }) + + test("resets the grace window when the tab becomes visible", async () => { + await provider.resolveWebviewView(mockWebviewPanel) + Object.defineProperty(mockWebviewPanel, "visible", { value: false, configurable: true }) + const htmlAfterResolve = mockWebviewPanel.webview.html + + await vi.advanceTimersByTimeAsync(120_000) + expect(mockWebviewPanel.webview.html).toBe(htmlAfterResolve) + + Object.defineProperty(mockWebviewPanel, "visible", { value: true, configurable: true }) + viewStateCallback() + const htmlAfterBecomingVisible = mockWebviewPanel.webview.html + + await vi.advanceTimersByTimeAsync(60_000) + expect(mockWebviewPanel.webview.html).toBe(htmlAfterBecomingVisible) + + // Watchdog ticks every 60s; 120s after the flip the heartbeat is stale again. + await vi.advanceTimersByTimeAsync(60_000) + expect(mockWebviewPanel.webview.html).not.toBe(htmlAfterBecomingVisible) + }) + }) }) test("resolveWebviewView sets up webview correctly in development mode even if local server is not running", async () => { From ba72a4e3ee3483ec8ba4fc24f680fc45d30419e1 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Wed, 23 Sep 2026 06:37:38 +0900 Subject: [PATCH 06/18] test(webview): cover recovery rejection and watchdog precondition Address two minor review threads on the watchdog spec: - Cover the reload rejection path: stub the recovery HTML regeneration to reject and assert the failure is logged and webview.html is untouched. - Assert the watchdog interval was actually scheduled before the sidebar disposal test disposes the view, so the post-disposal null check proves the watchdog was stopped rather than never having started. --- .../webview/__tests__/ClineProvider.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 88cf2977e2..d19298dda7 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -749,6 +749,11 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html + // Precondition: the watchdog interval was actually scheduled, so the + // null check below proves disposal stopped it rather than it never + // having started. + expect(provider["webviewWatchdogInterval"]).not.toBeNull() + // The provider outlives a disposed sidebar view; VS Code re-resolves // a fresh view later. Disposal must stop the watchdog and drop the // stale view reference so no recovery reload targets the dead view. @@ -787,6 +792,22 @@ describe("ClineProvider", () => { ) }) + test("logs and keeps the webview html when the recovery reload rejects", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + // The watchdog reload no longer goes through a VS Code command; stub + // the recovery HTML regeneration itself to reject. + provider["getWebviewHtml"] = vi.fn().mockRejectedValue(new Error("reload boom")) + ;(mockOutputChannel.appendLine as ReturnType).mockClear() + + await vi.advanceTimersByTimeAsync(120_000) + + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("[Zoo Code] Failed to reload webview: reload boom"), + ) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + test("does not reassign html when the provider is disposed mid-recovery", async () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html From 1e6fd39684d5321af0790b2e74d064ad0621e852 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Sun, 27 Sep 2026 22:25:51 +0900 Subject: [PATCH 07/18] test(webview): assert global webview reload command is not used in provider isolation What: spy on vscode.commands.executeCommand in the multi-provider watchdog isolation test and assert workbench.action.webview.reloadWebviewAction was never invoked while the stale-heartbeat watchdog fires. Why: the pre-PR recovery path fired that global command, which resets every webview. The html-only assertions still pass under a regression back to the global command because the vscode mock swallows executeCommand without touching any view, so the test could not fail for the exact bug class it guards (code-reviewer P2 F-01 on PR #1715). --- src/core/webview/__tests__/ClineProvider.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index d19298dda7..fb392f3eba 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -920,6 +920,14 @@ describe("ClineProvider", () => { const htmlBeforeA = mockWebviewView.webview.html const htmlBeforeB = mockWebviewViewB.webview.html + // The pre-PR reload path fired the global + // workbench.action.webview.reloadWebviewAction command, which + // resets every webview. The html assertions below cannot catch a + // regression back to that command because the vscode mock + // swallows executeCommand without touching any view, so also + // assert the global command was never invoked. + const executeCommandMock = vscode.commands.executeCommand as ReturnType + await vi.advanceTimersByTimeAsync(119_000) // B's view is still alive; refresh its heartbeat so a stale one // would reload it too, leaving only A's watchdog to fire. @@ -928,6 +936,7 @@ describe("ClineProvider", () => { expect(mockWebviewView.webview.html).not.toBe(htmlBeforeA) expect(mockWebviewViewB.webview.html).toBe(htmlBeforeB) + expect(executeCommandMock).not.toHaveBeenCalledWith("workbench.action.webview.reloadWebviewAction") } finally { await providerB.dispose() } From da8d752ffb35c0861403899b41929a047df7c533 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Mon, 28 Sep 2026 00:09:27 +0900 Subject: [PATCH 08/18] fix(webview): harden recovery reload and scope sidebar disposal cleanup What: The recovery reload captures lastWebviewHeartbeatAt when recovery starts and, after the awaited HTML generation, additionally requires the heartbeat timestamp to be unchanged and the captured view to still be visible before reassigning webview.html. The sidebar onDidDispose handler now runs clearWebviewResources() and the codeIndexManager reset only when the disposed view is the currently watched one. Cancellation specs defer getWebviewHtml() itself (instead of getState() with a fabricated state) so they prove disposal and replacement land while recovery is in flight, and new specs cover heartbeat-during-recovery and hide-during-recovery skips. Why: A heartbeat arriving during recovery means the renderer is alive again, and a view that hides mid-recovery no longer proves a dead renderer (hidden webviews throttle timers), so reloading in either case needlessly reloads a healthy or throttled webview. This mitigates the security audit finding LOW-1 (self-reload loop when the postMessage bridge is half-broken): a renderer that resumes heartbeating during the recovery window is no longer reloaded on every watchdog tick. Disposing an outdated (replaced) sidebar view previously wiped the replacement view's resources and code index manager; scoping the cleanup to the watched view keeps the replacement intact. --- src/core/webview/ClineProvider.ts | 27 +++++-- .../webview/__tests__/ClineProvider.spec.ts | 79 +++++++++++++++++-- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4be16135c2..811ab40e3f 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1146,15 +1146,15 @@ export class ClineProvider this.log("Disposing ClineProvider instance for tab view") await this.dispose() } else { - this.log("Clearing webview resources for sidebar view") - this.clearWebviewResources() if (this.view === webviewView) { + this.log("Clearing webview resources for sidebar view") + this.clearWebviewResources() // Drop the disposed view so nothing keeps polling it // (e.g. the recovery watchdog) for the provider's lifetime. this.view = undefined + // Reset current workspace manager reference when view is disposed + this.codeIndexManager = undefined } - // Reset current workspace manager reference when view is disposed - this.codeIndexManager = undefined } }, null, @@ -3452,12 +3452,23 @@ export class ClineProvider // Capture the epoch so a disposal or view replacement can invalidate // this operation while the HTML is being generated. const epoch = this.webviewRecoveryEpoch + // A heartbeat that arrives while the HTML is being generated means the + // renderer is alive again; comparing against the timestamp captured here + // lets the post-await check skip the reload in that case. + const heartbeatAtRecoveryStart = this.lastWebviewHeartbeatAt try { const html = await this.getWebviewHtml(view.webview) - // The await yields; assigning html now that the provider is disposed - // or the watched view was disposed/replaced would touch a dead or - // unrelated webview. - if (this._disposed || this.webviewRecoveryEpoch !== epoch || this.view !== view) { + // The await yields; assigning html now that the provider is disposed, + // the watched view was disposed/replaced, the renderer heartbeat + // recovered, or the view hid again would either touch a dead or + // unrelated webview or reload one that no longer needs recovery. + if ( + this._disposed || + this.webviewRecoveryEpoch !== epoch || + this.view !== view || + this.lastWebviewHeartbeatAt !== heartbeatAtRecoveryStart || + view.visible !== true + ) { return } view.webview.html = html diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index fb392f3eba..79f0168365 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -837,19 +837,24 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html - let finishReload: (state: ExtensionState) => void = () => {} - vi.spyOn(provider, "getState").mockImplementation( + // Defer the recovery reload's own HTML generation so the test proves + // the disposal lands while recovery is in flight, not before it starts. + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( () => - new Promise((resolve) => { + new Promise((resolve) => { finishReload = resolve }), ) await vi.advanceTimersByTimeAsync(120_000) + // The stale heartbeat started a recovery reload, and its HTML + // generation is still pending. + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) disposeCallback() - finishReload({ apiConfiguration: {} } as unknown as ExtensionState) + finishReload("recovered") await vi.advanceTimersByTimeAsync(0) expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) @@ -859,15 +864,21 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html - let finishReload: (state: ExtensionState) => void = () => {} - vi.spyOn(provider, "getState").mockImplementation( + // Defer the recovery reload's own HTML generation so the test proves + // the replacement lands while recovery is in flight, not before it + // starts. + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( () => - new Promise((resolve) => { + new Promise((resolve) => { finishReload = resolve }), ) await vi.advanceTimersByTimeAsync(120_000) + // The stale heartbeat started a recovery reload, and its HTML + // generation is still pending. + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) // VS Code re-resolves a fresh view (e.g. sidebar re-opened) while the @@ -885,7 +896,59 @@ describe("ClineProvider", () => { visible: true, } - finishReload({ apiConfiguration: {} } as unknown as ExtensionState) + finishReload("recovered") + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + + test("skips the recovery reload when a heartbeat arrives while regenerating HTML", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + // The renderer process reported in again while the recovery HTML was + // still being generated, so the webview is alive and must not reload. + provider.updateWebviewHeartbeat() + + finishReload("recovered") + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + + test("skips the recovery reload when the view hides while regenerating HTML", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + + // The view hid while the recovery HTML was still being generated; a + // hidden webview throttles heartbeats, so the stale heartbeat no + // longer proves a dead renderer. + Object.defineProperty(mockWebviewView, "visible", { value: false, configurable: true }) + + finishReload("recovered") await vi.advanceTimersByTimeAsync(0) expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) From 148b6416b609e9806f5bffe341891f4fc443899e Mon Sep 17 00:00:00 2001 From: myk1yt Date: Mon, 28 Sep 2026 01:23:35 +0900 Subject: [PATCH 09/18] test(e2e): poll restart task-history presence before asserting What: runVerify now gates the Task should be present after restart assertion on a bounded waitForTaskInHistory poll that uses the same api.isTaskInHistory read as the assertion, instead of a single unguarded read immediately after isReady. Why: the e2e-mock lane failed on da8d752ff with exactly this assertion (false !== true) while the same commit passed 6/6 local runs and the changed production paths cannot execute during the scenario (watchdog tick 60s vs ~2s phases; the sidebar dispose scoping is behavior-identical for the single watched view). The read races the persisted state becoming observable on a fresh host, the same swap-window failure class documented in #1641 for this scenario, which #1663 fixed for the conversation-history reads but left the isTaskInHistory read single-shot. Polling with the same read removes the false negative without touching production code, keeping the CodeRabbit-requested recovery and dispose guarantees intact. --- apps/vscode-e2e/src/suite/restart-persistence.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/vscode-e2e/src/suite/restart-persistence.test.ts b/apps/vscode-e2e/src/suite/restart-persistence.test.ts index 699b64e9cc..902dc25f51 100644 --- a/apps/vscode-e2e/src/suite/restart-persistence.test.ts +++ b/apps/vscode-e2e/src/suite/restart-persistence.test.ts @@ -35,6 +35,14 @@ async function waitForMarkedCompletion(api: RooCodeAPI, taskId: string): Promise ) } +async function waitForTaskInHistory(api: RooCodeAPI, taskId: string): Promise { + // Poll the same read the assertion uses. A single read can observe the + // persisted state before it is visible on a fresh host (the atomic-rename + // swap window documented in #1641), so gate the assertion on a bounded + // wait instead of failing on the first miss. + await waitFor(() => api.isTaskInHistory(taskId)) +} + async function runCreate(api: RooCodeAPI): Promise { let taskId: string | undefined let createPhasePassed = false @@ -92,6 +100,7 @@ async function runVerify(api: RooCodeAPI): Promise { api.on(RooCodeEventName.Message, messageHandler) await waitFor(() => api.isReady()) + await waitForTaskInHistory(api, taskId) assert.strictEqual(await api.isTaskInHistory(taskId), true, "Task should be present after restart") const historyItem = await api.getTaskHistoryItem(taskId) assert.ok(historyItem, "Task history item should be available after restart") From 54f6a93d57fed5e3d4337106c093edfd63a144c7 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Mon, 28 Sep 2026 01:50:12 +0900 Subject: [PATCH 10/18] fix(webview): guard recovery with heartbeat revision and in-flight flag What: The recovery reload now compares a webviewHeartbeatRevision counter (captured before the awaited HTML generation against the value after) instead of comparing heartbeat timestamps, and reloadWebviewForRecovery skips starting when webviewRecoveryInFlight is already set, clearing that flag in a finally block so a failed generation cannot block future recoveries. updateWebviewHeartbeat bumps the revision on every accepted heartbeat. New specs cover the same-millisecond double-heartbeat case and the no-second-recovery-while-in-flight case including the reject path. Why: Two heartbeats landing in the same millisecond advanced the old timestamp not at all, so a recovery that started before them could still replace a page whose renderer had reported back twice; a monotonic counter catches every accepted heartbeat. Without the in-flight flag, every 60s watchdog tick during a slow HTML generation started another recovery for the same view, piling forced reloads onto it; the flag limits the view to one recovery at a time while the finally block keeps one failed attempt from disabling recovery permanently. The existing webviewRecoveryEpoch checks stay in place: the epoch invalidates completions after dispose or view replacement, while the in-flight flag only serializes generation starts, so the two guards address different races and both remain. --- src/core/webview/ClineProvider.ts | 24 +++++-- .../webview/__tests__/ClineProvider.spec.ts | 62 +++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 811ab40e3f..69a8cf0614 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -224,11 +224,18 @@ export class ClineProvider private currentWorkspacePath: string | undefined private _disposed = false private lastWebviewHeartbeatAt = 0 + // Bumped on every accepted heartbeat, including multiple heartbeats in the + // same millisecond; the recovery reload compares revisions around its + // awaited HTML generation so no accepted heartbeat is missed. + private webviewHeartbeatRevision = 0 private webviewWatchdogInterval: ReturnType | null = null // Bumped to invalidate an in-flight recovery reload when the provider or // the watched view is disposed; the reload must not reassign webview.html // afterwards. private webviewRecoveryEpoch = 0 + // True while a recovery reload is awaiting HTML generation; the watchdog + // must not pile a second reload onto the same view until this one settles. + private webviewRecoveryInFlight = false private static readonly WEBVIEW_WATCHDOG_TICK_MS = 60_000 private static readonly WEBVIEW_HEARTBEAT_STALE_MS = 90_000 private readonly _postStateToWebviewThrottled = debounce( @@ -3407,6 +3414,7 @@ export class ClineProvider /** Records that the webview renderer is alive; called on every webviewHeartbeat message. */ public updateWebviewHeartbeat(): void { this.lastWebviewHeartbeatAt = Date.now() + this.webviewHeartbeatRevision++ } /** @@ -3449,13 +3457,19 @@ export class ClineProvider if (!view?.webview) { return } + // A recovery already awaiting its HTML generation owns this view; a + // second one would only pile another forced reload onto it. + if (this.webviewRecoveryInFlight) { + return + } + this.webviewRecoveryInFlight = true // Capture the epoch so a disposal or view replacement can invalidate // this operation while the HTML is being generated. const epoch = this.webviewRecoveryEpoch // A heartbeat that arrives while the HTML is being generated means the - // renderer is alive again; comparing against the timestamp captured here - // lets the post-await check skip the reload in that case. - const heartbeatAtRecoveryStart = this.lastWebviewHeartbeatAt + // renderer is alive again; comparing revisions (not timestamps) lets the + // post-await check catch heartbeats that land in the same millisecond. + const heartbeatRevision = this.webviewHeartbeatRevision try { const html = await this.getWebviewHtml(view.webview) // The await yields; assigning html now that the provider is disposed, @@ -3466,7 +3480,7 @@ export class ClineProvider this._disposed || this.webviewRecoveryEpoch !== epoch || this.view !== view || - this.lastWebviewHeartbeatAt !== heartbeatAtRecoveryStart || + this.webviewHeartbeatRevision !== heartbeatRevision || view.visible !== true ) { return @@ -3474,6 +3488,8 @@ export class ClineProvider view.webview.html = html } catch (error) { this.log(`[Zoo Code] Failed to reload webview: ${error instanceof Error ? error.message : String(error)}`) + } finally { + this.webviewRecoveryInFlight = false } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 79f0168365..c9c9d8ce31 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -928,6 +928,36 @@ describe("ClineProvider", () => { expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) + test("skips the recovery reload when two heartbeats land in the same millisecond during recovery", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + // Pin the clock so both heartbeats stamp the same millisecond: a + // timestamp comparison would see no change, but the heartbeat + // revision moved, so the reload must still be skipped. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(Date.now()) + provider.updateWebviewHeartbeat() + provider.updateWebviewHeartbeat() + nowSpy.mockRestore() + + finishReload("recovered") + await vi.advanceTimersByTimeAsync(0) + + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + }) + test("skips the recovery reload when the view hides while regenerating HTML", async () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html @@ -954,6 +984,38 @@ describe("ClineProvider", () => { expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) }) + test("does not start a second recovery while one is in flight and recovers after a failure", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + // The first recovery blocks on a controlled pending generation. + let failReload: (error: Error) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((_resolve, reject) => { + failReload = reject + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + + // The watchdog ticks again while the first recovery still awaits + // its HTML; no second generation may start for the same view. + await vi.advanceTimersByTimeAsync(60_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + + // The pending generation rejects; the finally block must clear the + // in-flight state so a later watchdog tick can start a fresh + // recovery instead of being blocked forever. + failReload(new Error("reload boom")) + await vi.advanceTimersByTimeAsync(0) + + await vi.advanceTimersByTimeAsync(60_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(2) + }) + test("reloads only its own webview when multiple providers are active", async () => { // Structural stand-in for the VS Code webview API surface this scenario // exercises, same as the mockContext cast below. From fad059e26e0e973460f74cef359f46c3aace51cf Mon Sep 17 00:00:00 2001 From: myk1yt Date: Mon, 28 Sep 2026 02:32:21 +0900 Subject: [PATCH 11/18] fix(webview): scope recovery in-flight guard to epoch What: The in-flight guard for recovery reloads now records which webviewRecoveryEpoch owns the pending recovery (webviewRecoveryInFlightEpoch) instead of a plain boolean. A new recovery is only blocked when the pending one belongs to the same epoch, and the finally block releases ownership only if the completing recovery still holds it. Specs: the same-millisecond heartbeat spec captures the pre-spy timestamp, feeds it to the Date.now spy, and asserts the stamp is unchanged after each heartbeat; the retry spec resolves the second getWebviewHtml call with distinct HTML and asserts it lands in the webview; a new spec covers recovery A pending, view disposed/replaced (epoch bump), recovery B starting immediately, and A's finally not releasing B's ownership. Why: With a boolean flag, a recovery pending on a disposed or replaced view blocked the replacement view's recovery until the stale generation settled, delaying recovery of the live view by a whole generation window. Scoping ownership to the epoch keeps the pile-up protection for the same view while letting the replacement's recovery start immediately; the conditional finally clear stops a stale completion from releasing ownership the replacement recovery took. The per-heartbeat timestamp assertions prove both same-millisecond heartbeats leave the recovery-start stamp unchanged, which is exactly the case the revision counter exists for, and the retry HTML assertion proves a post-failure recovery actually replaces the page content. --- src/core/webview/ClineProvider.ts | 28 +++-- .../webview/__tests__/ClineProvider.spec.ts | 102 +++++++++++++++--- 2 files changed, 107 insertions(+), 23 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 69a8cf0614..eac211fe89 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -233,9 +233,11 @@ export class ClineProvider // the watched view is disposed; the reload must not reassign webview.html // afterwards. private webviewRecoveryEpoch = 0 - // True while a recovery reload is awaiting HTML generation; the watchdog - // must not pile a second reload onto the same view until this one settles. - private webviewRecoveryInFlight = false + // Epoch that owns an in-flight recovery reload, if any. Only a recovery + // from the same epoch is blocked, so disposing/replacing the view (which + // bumps webviewRecoveryEpoch) never makes the replacement wait on the + // stale recovery's completion. + private webviewRecoveryInFlightEpoch: number | undefined = undefined private static readonly WEBVIEW_WATCHDOG_TICK_MS = 60_000 private static readonly WEBVIEW_HEARTBEAT_STALE_MS = 90_000 private readonly _postStateToWebviewThrottled = debounce( @@ -3457,15 +3459,17 @@ export class ClineProvider if (!view?.webview) { return } - // A recovery already awaiting its HTML generation owns this view; a - // second one would only pile another forced reload onto it. - if (this.webviewRecoveryInFlight) { - return - } - this.webviewRecoveryInFlight = true // Capture the epoch so a disposal or view replacement can invalidate // this operation while the HTML is being generated. const epoch = this.webviewRecoveryEpoch + // A recovery already awaiting its HTML generation owns this view; a + // second one would only pile another forced reload onto it. Ownership + // is scoped to the epoch so a stale recovery (its view disposed or + // replaced, epoch bumped) never blocks the replacement view's recovery. + if (this.webviewRecoveryInFlightEpoch === epoch) { + return + } + this.webviewRecoveryInFlightEpoch = epoch // A heartbeat that arrives while the HTML is being generated means the // renderer is alive again; comparing revisions (not timestamps) lets the // post-await check catch heartbeats that land in the same millisecond. @@ -3489,7 +3493,11 @@ export class ClineProvider } catch (error) { this.log(`[Zoo Code] Failed to reload webview: ${error instanceof Error ? error.message : String(error)}`) } finally { - this.webviewRecoveryInFlight = false + // Clear ownership only while this completion still holds it; a + // stale recovery must not release the replacement's in-flight state. + if (this.webviewRecoveryInFlightEpoch === epoch) { + this.webviewRecoveryInFlightEpoch = undefined + } } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index c9c9d8ce31..776e12194e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -944,12 +944,16 @@ describe("ClineProvider", () => { expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) - // Pin the clock so both heartbeats stamp the same millisecond: a - // timestamp comparison would see no change, but the heartbeat - // revision moved, so the reload must still be skipped. - const nowSpy = vi.spyOn(Date, "now").mockReturnValue(Date.now()) + // Pin the clock to the timestamp captured before recovery: both + // heartbeats stamp the same millisecond, so a timestamp comparison + // would see no change, but the heartbeat revision moved, so the + // reload must still be skipped. + const heartbeatAtCapture = provider["lastWebviewHeartbeatAt"] + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(heartbeatAtCapture) provider.updateWebviewHeartbeat() + expect(provider["lastWebviewHeartbeatAt"]).toBe(heartbeatAtCapture) provider.updateWebviewHeartbeat() + expect(provider["lastWebviewHeartbeatAt"]).toBe(heartbeatAtCapture) nowSpy.mockRestore() finishReload("recovered") @@ -988,14 +992,24 @@ describe("ClineProvider", () => { await provider.resolveWebviewView(mockWebviewView) const htmlAfterResolve = mockWebviewView.webview.html - // The first recovery blocks on a controlled pending generation. - let failReload: (error: Error) => void = () => {} - provider["getWebviewHtml"] = vi.fn().mockImplementation( - () => - new Promise((_resolve, reject) => { - failReload = reject - }), - ) + // The first recovery blocks on a controlled pending generation; the + // retry (second call) gets its own deferred generation. + let failFirst: (error: Error) => void = () => {} + let finishRetry: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi + .fn() + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + failFirst = reject + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishRetry = resolve + }), + ) await vi.advanceTimersByTimeAsync(120_000) expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) @@ -1009,11 +1023,73 @@ describe("ClineProvider", () => { // The pending generation rejects; the finally block must clear the // in-flight state so a later watchdog tick can start a fresh // recovery instead of being blocked forever. - failReload(new Error("reload boom")) + failFirst(new Error("reload boom")) await vi.advanceTimersByTimeAsync(0) await vi.advanceTimersByTimeAsync(60_000) expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(2) + + // The retry's generation resolves with fresh HTML and must land in + // the webview. + finishRetry("recovered-retry") + await vi.advanceTimersByTimeAsync(0) + expect(mockWebviewView.webview.html).toContain("recovered-retry") + }) + + test("lets the replacement view's recovery start while a stale one is pending", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + + // Each recovery generation gets its own deferred finish callback. + const generations: Array<{ finish: (html: string) => void }> = [] + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + generations.push({ finish: resolve }) + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + + // The sidebar view is disposed (bumping webviewRecoveryEpoch) and a + // replacement view takes over while recovery A's HTML generation is + // still pending, making A stale. + disposeCallback() + // @ts-ignore - accessing private property for testing + provider.view = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + } + const replacementEpoch = provider["webviewRecoveryEpoch"] + + // Recovery B for the new view must start even though A is still + // pending: the in-flight guard is scoped to the epoch. + void provider["reloadWebviewForRecovery"]() + await vi.advanceTimersByTimeAsync(0) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(2) + + // A's completion is stale: it must not reassign the old view's html, + // and its finally must NOT release the ownership B took. + generations[0].finish("stale-A") + await vi.advanceTimersByTimeAsync(0) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + expect(provider["webviewRecoveryInFlightEpoch"]).toBe(replacementEpoch) + + // B completes and lands its HTML on the new view, then releases + // ownership so future recoveries can start. + generations[1].finish("recovered-B") + await vi.advanceTimersByTimeAsync(0) + // @ts-ignore - accessing private property for testing + expect(provider.view.webview.html).toContain("recovered-B") + expect(provider["webviewRecoveryInFlightEpoch"]).toBeUndefined() }) test("reloads only its own webview when multiple providers are active", async () => { From 939e85ff92d97950ca2a41310a3b7017f8369554 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Mon, 28 Sep 2026 06:12:19 +0900 Subject: [PATCH 12/18] fix(webview): bump recovery epoch when resolveWebviewView replaces the view What: resolveWebviewView now advances webviewRecoveryEpoch (the same increment clearWebviewResources uses) when the incoming view is a different object than the currently watched one, before assigning this.view. Re-resolving the same view and the first resolve leave the epoch unchanged. New specs: a replacement resolve without an intervening disposal advances the epoch, lets the replacement view's recovery start while the stale one is still pending, keeps the stale completion from reassigning the old view's html or releasing the replacement's in-flight ownership, and lands the replacement's HTML; a same-view re-resolve asserts no epoch bump and that the pending recovery's ownership still blocks a second recovery until it settles. Why: Without the bump, a recovery pending on the old view kept the epoch-scoped in-flight ownership, so a replacement view resolved without an intervening disposal (a path the dispose handler never saw) could not start its own recovery until the stale generation settled, delaying the live view's recovery by a whole generation window. The bump closes that interleaving while leaving the normal dispose-then-resolve path unchanged: disposal already bumps via clearWebviewResources, and by the time the replacement resolves this.view is undefined, so the two bump sites never double-count one replacement. Same-view re-resolves (a known VS Code re-resolve pattern) must not invalidate an in-flight recovery, so they are excluded from the bump. --- src/core/webview/ClineProvider.ts | 8 ++ .../webview/__tests__/ClineProvider.spec.ts | 112 ++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index eac211fe89..5e964998bd 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1042,6 +1042,14 @@ export class ClineProvider } async resolveWebviewView(webviewView: vscode.WebviewView | vscode.WebviewPanel) { + // Replacing the watched view invalidates a recovery reload still + // awaiting its HTML (same epoch bump as clearWebviewResources), so the + // stale recovery can neither block nor reassign the replacement view's + // recovery. Re-resolving the same view, or the first resolve, leaves + // the epoch alone. + if (this.view && this.view !== webviewView) { + this.webviewRecoveryEpoch++ + } this.view = webviewView const inTabMode = "onDidChangeViewState" in webviewView diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 776e12194e..ccc7136118 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1092,6 +1092,118 @@ describe("ClineProvider", () => { expect(provider["webviewRecoveryInFlightEpoch"]).toBeUndefined() }) + test("advances the recovery epoch when resolveWebviewView replaces the view and lets the new recovery start", async () => { + await provider.resolveWebviewView(mockWebviewView) + const htmlAfterResolve = mockWebviewView.webview.html + const epochBeforeReplacement = provider["webviewRecoveryEpoch"] + + // Recovery generations get their own deferred finish callbacks; the + // replacement view's initial HTML generation resolves immediately so + // resolveWebviewView can complete. + const generations: Array<{ finish: (html: string) => void }> = [] + provider["getWebviewHtml"] = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + generations.push({ finish: resolve }) + }), + ) + .mockImplementationOnce(() => Promise.resolve("view2-initial")) + .mockImplementation( + () => + new Promise((resolve) => { + generations.push({ finish: resolve }) + }), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + + // VS Code re-resolves a DIFFERENT view without a disposal in between + // (the gap path): the epoch must advance so the stale recovery can + // neither block nor reassign the replacement view's recovery. + const mockWebviewView2 = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi.fn(), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn(), + onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + } as unknown as vscode.WebviewView + await provider.resolveWebviewView(mockWebviewView2) + expect(provider["webviewRecoveryEpoch"]).toBe(epochBeforeReplacement + 1) + + // Recovery B for the replacement view starts immediately, unblocked + // by A's still-pending generation. + void provider["reloadWebviewForRecovery"]() + await vi.advanceTimersByTimeAsync(0) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(3) + + // A's completion is stale: the old view's html is untouched and A's + // finally does not release the ownership B took. + generations[0].finish("stale-A") + await vi.advanceTimersByTimeAsync(0) + expect(mockWebviewView.webview.html).toBe(htmlAfterResolve) + expect(provider["webviewRecoveryInFlightEpoch"]).toBe(provider["webviewRecoveryEpoch"]) + + // B completes and lands its HTML on the replacement view, then + // releases ownership. + generations[1].finish("recovered-B") + await vi.advanceTimersByTimeAsync(0) + expect(mockWebviewView2.webview.html).toContain("recovered-B") + expect(provider["webviewRecoveryInFlightEpoch"]).toBeUndefined() + }) + + test("does not advance the recovery epoch when the same view is re-resolved and keeps the pending recovery intact", async () => { + await provider.resolveWebviewView(mockWebviewView) + const epochAfterFirstResolve = provider["webviewRecoveryEpoch"] + + // Recovery A blocks on a controlled pending generation; the same + // view's re-resolve initial HTML resolves immediately. + let finishReload: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishReload = resolve + }), + ) + .mockImplementation(() => + Promise.resolve("same-view-reresolve"), + ) + + await vi.advanceTimersByTimeAsync(120_000) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(1) + + // Re-resolving the SAME view must not bump the epoch nor disrupt the + // pending recovery that belongs to it. + await provider.resolveWebviewView(mockWebviewView) + expect(provider["webviewRecoveryEpoch"]).toBe(epochAfterFirstResolve) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(2) + expect(provider["webviewRecoveryInFlightEpoch"]).toBe(epochAfterFirstResolve) + + // A still owns the view across the re-resolve: another recovery + // attempt is blocked until A settles. + void provider["reloadWebviewForRecovery"]() + await vi.advanceTimersByTimeAsync(0) + expect(provider["getWebviewHtml"]).toHaveBeenCalledTimes(2) + + // A settles cleanly: the re-resolve restarted the watchdog, which + // re-stamped the heartbeat, so the revision guard (not the epoch) + // skips the stale assignment; the ownership A held is released. + finishReload("recovered-A") + await vi.advanceTimersByTimeAsync(0) + expect(mockWebviewView.webview.html).toContain("same-view-reresolve") + expect(provider["webviewRecoveryInFlightEpoch"]).toBeUndefined() + }) + test("reloads only its own webview when multiple providers are active", async () => { // Structural stand-in for the VS Code webview API surface this scenario // exercises, same as the mockContext cast below. From 1c9ac529b2d278f030040306d5eb26f60fddb7df Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 29 Sep 2026 00:55:36 +0900 Subject: [PATCH 13/18] chore(e2e): restore restart-persistence test to merge-base state What: Restores apps/vscode-e2e/src/suite/restart-persistence.test.ts to the merge-base (08d05eb0f), removing the waitForTaskInHistory polling helper and its call in runVerify that commit 148b6416b added. Why: The restart-persistence polling change is unrelated to webview renderer heartbeat recovery and was flagged as an out-of-scope change in the PR's pre-merge review; it will be submitted separately. Impact: The PR diff no longer touches apps/vscode-e2e; no runtime behavior changes. --- apps/vscode-e2e/src/suite/restart-persistence.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/apps/vscode-e2e/src/suite/restart-persistence.test.ts b/apps/vscode-e2e/src/suite/restart-persistence.test.ts index 902dc25f51..699b64e9cc 100644 --- a/apps/vscode-e2e/src/suite/restart-persistence.test.ts +++ b/apps/vscode-e2e/src/suite/restart-persistence.test.ts @@ -35,14 +35,6 @@ async function waitForMarkedCompletion(api: RooCodeAPI, taskId: string): Promise ) } -async function waitForTaskInHistory(api: RooCodeAPI, taskId: string): Promise { - // Poll the same read the assertion uses. A single read can observe the - // persisted state before it is visible on a fresh host (the atomic-rename - // swap window documented in #1641), so gate the assertion on a bounded - // wait instead of failing on the first miss. - await waitFor(() => api.isTaskInHistory(taskId)) -} - async function runCreate(api: RooCodeAPI): Promise { let taskId: string | undefined let createPhasePassed = false @@ -100,7 +92,6 @@ async function runVerify(api: RooCodeAPI): Promise { api.on(RooCodeEventName.Message, messageHandler) await waitFor(() => api.isReady()) - await waitForTaskInHistory(api, taskId) assert.strictEqual(await api.isTaskInHistory(taskId), true, "Task should be present after restart") const historyItem = await api.getTaskHistoryItem(taskId) assert.ok(historyItem, "Task history item should be available after restart") From aa666fc402504e5f89337fda3238dfe655654162 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 29 Sep 2026 01:12:31 +0900 Subject: [PATCH 14/18] fix(webview): gate obsolete-view messages and dispose replaced-view subscriptions What: setWebviewMessageListener now takes the resolved view/panel and skips dispatch when this.view no longer references it, so a replaced view's live renderer cannot post into the provider. resolveWebviewView now tracks the view-specific subscriptions (message, visibility, active-editor, and configuration listeners plus the view's onDidDispose registration) in a dedicated resolvedViewDisposables array; switching to a different view disposes the previous entries before installing the replacement's, and both visibility callbacks (onDidChangeViewState and onDidChangeVisibility) also guard on this.view === webviewView before refreshing the provider-wide heartbeat. clearWebviewResources disposes the per-view subscriptions too, so provider dispose() and sidebar disposal still clean up everything. New specs: a replaced view's message is ignored while the current view's dispatches; the replaced view's subscriptions are each disposed exactly once and only the replacement's remain; a replaced sidebar view's late dispose callback neither stops the replacement's watchdog nor blocks its stale-heartbeat recovery; a replaced tab panel's view state changes and messages are ignored while the current panel's still dispatch; and a racing stale visibility event cannot refresh the heartbeat. Why: A live old renderer kept the provider-wide heartbeat timestamp fresh after its view was replaced, so the dead-renderer watchdog could skip recovery and leave the current webview gray, and every resolution accumulated duplicate listeners in the shared webviewDisposables array. Impact: Only ClineProvider and its spec change; same-view re-resolves keep today's behavior (no disposal, no epoch bump), and the existing sidebar onDidDispose guard semantics are unchanged. --- src/core/webview/ClineProvider.ts | 62 +++++-- .../webview/__tests__/ClineProvider.spec.ts | 166 ++++++++++++++++++ 2 files changed, 216 insertions(+), 12 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 5e964998bd..2422b4c724 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -198,6 +198,12 @@ export class ClineProvider private static activeInstances: Set = new Set() private disposables: vscode.Disposable[] = [] private webviewDisposables: vscode.Disposable[] = [] + // Subscriptions tied to the currently resolved view (message, visibility, + // active-editor, and configuration listeners plus the view's disposal + // registration). Replacing the view disposes the previous entries before + // the replacement's are installed, so a stale view can neither dispatch + // messages nor refresh the provider-wide heartbeat. + private resolvedViewDisposables: vscode.Disposable[] = [] private pendingThemeFixtureProbes = new Map< string, { @@ -837,6 +843,7 @@ export class ClineProvider // Invalidate any recovery reload still awaiting its HTML so it cannot // reassign webview.html on the disposed view. this.webviewRecoveryEpoch++ + this.disposeResolvedViewResources() while (this.webviewDisposables.length) { const x = this.webviewDisposables.pop() if (x) { @@ -845,6 +852,16 @@ export class ClineProvider } } + /** Disposes the subscriptions registered for the previously resolved view. */ + private disposeResolvedViewResources() { + while (this.resolvedViewDisposables.length) { + const x = this.resolvedViewDisposables.pop() + if (x) { + x.dispose() + } + } + } + /** Drain one task's memoized cleanup without preventing the remaining provider shutdown work. */ private async drainTaskDisposal(task: Task): Promise { try { @@ -1049,6 +1066,11 @@ export class ClineProvider // the epoch alone. if (this.view && this.view !== webviewView) { this.webviewRecoveryEpoch++ + // The replaced view's listeners must not outlive it: dispose them + // before the replacement's subscriptions are installed below so no + // duplicate message/visibility/editor/configuration listeners + // accumulate across A-to-B replacements. + this.disposeResolvedViewResources() } this.view = webviewView const inTabMode = "onDidChangeViewState" in webviewView @@ -1106,7 +1128,7 @@ export class ClineProvider // Sets up an event listener to listen for messages passed from the webview view context // and executes code based on the message that is received. - this.setWebviewMessageListener(webviewView.webview) + this.setWebviewMessageListener(webviewView) // Detect a dead webview renderer process (gray screen) via heartbeat timeout. this.startWebviewWatchdog() @@ -1120,7 +1142,7 @@ export class ClineProvider // Update subscription when workspace might have changed. this.updateCodeIndexStatusSubscription() }) - this.webviewDisposables.push(activeEditorSubscription) + this.resolvedViewDisposables.push(activeEditorSubscription) // Listen for when the panel becomes visible. // https://github.com/microsoft/vscode-discussions/discussions/840 @@ -1128,6 +1150,11 @@ export class ClineProvider // WebviewView and WebviewPanel have all the same properties except // for this visibility listener panel. const viewStateDisposable = webviewView.onDidChangeViewState(() => { + if (this.view !== webviewView) { + // A replaced view must not refresh the provider-wide + // heartbeat the current view's watchdog relies on. + return + } if (this.view?.visible) { // Hidden webviews throttle timers, so grant a fresh grace // window instead of counting throttled heartbeats as a crash. @@ -1138,10 +1165,15 @@ export class ClineProvider } }) - this.webviewDisposables.push(viewStateDisposable) + this.resolvedViewDisposables.push(viewStateDisposable) } else if ("onDidChangeVisibility" in webviewView) { // sidebar const visibilityDisposable = webviewView.onDidChangeVisibility(() => { + if (this.view !== webviewView) { + // A replaced view must not refresh the provider-wide + // heartbeat the current view's watchdog relies on. + return + } if (this.view?.visible) { // Hidden webviews throttle timers, so grant a fresh grace // window instead of counting throttled heartbeats as a crash. @@ -1152,7 +1184,7 @@ export class ClineProvider } }) - this.webviewDisposables.push(visibilityDisposable) + this.resolvedViewDisposables.push(visibilityDisposable) } // Listen for when the view is disposed @@ -1175,7 +1207,7 @@ export class ClineProvider } }, null, - this.disposables, + this.resolvedViewDisposables, ) // Listen for when color changes @@ -1185,7 +1217,7 @@ export class ClineProvider await this.postMessageToWebview({ type: "theme", text: JSON.stringify(await getTheme()) }) } }) - this.webviewDisposables.push(configDisposable) + this.resolvedViewDisposables.push(configDisposable) // If the extension is starting a new session, clear previous task state. // But don't clear if there's already an active task (e.g., resumed via IPC/bridge). @@ -1748,14 +1780,20 @@ export class ClineProvider * Sets up an event listener to listen for messages passed from the webview context and * executes code based on the message that is received. * - * @param webview A reference to the extension webview + * @param webviewView The resolved webview view or panel */ - private setWebviewMessageListener(webview: vscode.Webview) { - const onReceiveMessage = async (message: WebviewMessage) => - webviewMessageHandler(this, message, this.marketplaceManager) + private setWebviewMessageListener(webviewView: vscode.WebviewView | vscode.WebviewPanel) { + const onReceiveMessage = async (message: WebviewMessage) => { + // A replaced view's listener stays registered until the provider + // clears its subscriptions; never let a stale renderer dispatch. + if (this.view !== webviewView) { + return + } + await webviewMessageHandler(this, message, this.marketplaceManager) + } - const messageDisposable = webview.onDidReceiveMessage(onReceiveMessage) - this.webviewDisposables.push(messageDisposable) + const messageDisposable = webviewView.webview.onDidReceiveMessage(onReceiveMessage) + this.resolvedViewDisposables.push(messageDisposable) } /** diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index ccc7136118..44e7424e7b 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1255,6 +1255,172 @@ describe("ClineProvider", () => { } }) + describe("obsolete view gating and replacement cleanup", () => { + // Sidebar-shaped replacement view; callbacks are read through thunks + // because the mock assigns them when resolveWebviewView registers. + const createViewB = () => { + let messageCallbackB: (message: WebviewMessage) => Promise = async () => {} + const viewB = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi + .fn() + .mockImplementation((cb: (message: WebviewMessage) => Promise) => { + messageCallbackB = cb + return { dispose: vi.fn() } + }), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + onDidChangeVisibility: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + } as unknown as vscode.WebviewView + return { viewB, sendMessage: (message: WebviewMessage) => messageCallbackB(message) } + } + + test("ignores messages from a replaced view but still dispatches for the current view", async () => { + let messageCallbackA: (message: WebviewMessage) => Promise = async () => {} + mockWebviewView.webview.onDidReceiveMessage = vi + .fn() + .mockImplementation((cb: (message: WebviewMessage) => Promise) => { + messageCallbackA = cb + return { dispose: vi.fn() } + }) + await provider.resolveWebviewView(mockWebviewView) + const { viewB, sendMessage } = createViewB() + await provider.resolveWebviewView(viewB) + + const revisionBefore = provider["webviewHeartbeatRevision"] + // The stale renderer reports in; its dispatch must be skipped. + await messageCallbackA({ type: "webviewHeartbeat", timestamp: Date.now() }) + expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore) + + // The current view's dispatch reaches the handler as before. + await sendMessage({ type: "webviewHeartbeat", timestamp: Date.now() }) + expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore + 1) + }) + + test("disposes the replaced view's subscriptions so listeners do not accumulate", async () => { + const aDisposables: Array<{ dispose: ReturnType }> = [] + let visibilityCallbackA: () => void = () => {} + mockWebviewView.webview.onDidReceiveMessage = vi.fn().mockImplementation(() => { + const d = { dispose: vi.fn() } + aDisposables.push(d) + return d + }) + mockWebviewView.onDidChangeVisibility = vi.fn().mockImplementation((cb: () => void) => { + visibilityCallbackA = cb + const d = { dispose: vi.fn() } + aDisposables.push(d) + return d + }) + // Emulate the real onDidDispose(listener, thisArgs, disposables) + // contract so the disposal registration is tracked per view too. + mockWebviewView.onDidDispose = vi + .fn() + .mockImplementation((cb: () => void, _thisArgs: null, disposables?: vscode.Disposable[]) => { + const d = { dispose: vi.fn() } + aDisposables.push(d) + disposables?.push(d) + return d + }) + await provider.resolveWebviewView(mockWebviewView) + expect(aDisposables.length).toBe(3) // message, visibility, disposal registration + + const { viewB } = createViewB() + await provider.resolveWebviewView(viewB) + + expect(aDisposables.map((d) => d.dispose.mock.calls.length)).toEqual([1, 1, 1]) + // Only B's subscriptions remain: message, visibility, active + // editor, configuration. + expect(provider["resolvedViewDisposables"].length).toBe(4) + + // Even if a stale visibility event races in, it must not refresh + // the provider-wide heartbeat the current view's watchdog reads. + const revisionBefore = provider["webviewHeartbeatRevision"] + visibilityCallbackA() + expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore) + }) + + test("keeps the replacement view's watchdog and recovery when the replaced view's dispose callback fires", async () => { + let disposeCallbackA: () => void = () => {} + mockWebviewView.onDidDispose = vi.fn().mockImplementation((cb: () => void) => { + disposeCallbackA = cb + return { dispose: vi.fn() } + }) + await provider.resolveWebviewView(mockWebviewView) + const { viewB } = createViewB() + await provider.resolveWebviewView(viewB) + + // The stale view is finally torn down; its disposal must not + // clear the replacement's resources. + disposeCallbackA() + expect(provider["webviewWatchdogInterval"]).not.toBeNull() + // @ts-ignore - accessing private property for testing + expect(provider.view).toBe(viewB) + + // B remains eligible for recovery: once its heartbeat goes stale + // the watchdog reloads B's webview. + const htmlAfterBResolve = viewB.webview.html + await vi.advanceTimersByTimeAsync(120_000) + expect(viewB.webview.html).not.toBe(htmlAfterBResolve) + }) + + test("ignores a replaced tab panel's view state changes and messages", async () => { + // WebviewPanel-shaped views: visibility arrives via + // onDidChangeViewState instead of onDidChangeVisibility. + const createPanel = () => { + let viewStateCallback: () => void = () => {} + let messageCallback: (message: WebviewMessage) => Promise = async () => {} + const panel = { + webview: { + postMessage: vi.fn(), + html: "", + options: {}, + onDidReceiveMessage: vi + .fn() + .mockImplementation((cb: (message: WebviewMessage) => Promise) => { + messageCallback = cb + return { dispose: vi.fn() } + }), + asWebviewUri: vi.fn(), + cspSource: "vscode-webview://test-csp-source", + }, + visible: true, + onDidDispose: vi.fn().mockImplementation(() => ({ dispose: vi.fn() })), + onDidChangeViewState: vi.fn().mockImplementation((cb: () => void) => { + viewStateCallback = cb + return { dispose: vi.fn() } + }), + dispose: vi.fn(), + } as unknown as vscode.WebviewPanel + return { + panel, + fireViewState: () => viewStateCallback(), + sendMessage: (message: WebviewMessage) => messageCallback(message), + } + } + const panelA = createPanel() + await provider.resolveWebviewView(panelA.panel) + const panelB = createPanel() + await provider.resolveWebviewView(panelB.panel) + + // @ts-ignore - accessing private property for testing + expect(provider.view).toBe(panelB.panel) + + const revisionBefore = provider["webviewHeartbeatRevision"] + panelA.fireViewState() + await panelA.sendMessage({ type: "webviewHeartbeat", timestamp: Date.now() }) + expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore) + + panelB.fireViewState() + expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore + 1) + }) + }) + describe("tab panel (WebviewPanel shape)", () => { let viewStateCallback: () => void // Structural stand-in for the VS Code webview API surface this From 9f05e75d22d7a8a2c79673486f629fe8a3f5f450 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 29 Sep 2026 03:25:19 +0900 Subject: [PATCH 15/18] fix(webview): bail out of a resolve that went stale during its awaits What: resolveWebviewView now checks _disposed and this.view === webviewView after its getWebviewHtml/getState awaits and before installing the message listener, restarting the watchdog, or registering any other view subscription; a stale resolve returns without installing anything. New specs: a provider disposed mid-resolve ends with no watchdog interval and zero view subscriptions; a view that replaces a still-pending resolve ends with only the replacement's four subscriptions and the replacement keeping the watchdog. Why: Disposal (or a view replacement) during the awaited HTML/state generation used to stop the watchdog and clear resources, after which the pending resolve resumed and installed a fresh watchdog interval plus listeners for a disposed provider or a replaced view. That interval retained the provider and kept attempting recovery forever, and a replaced pending resolve duplicated the replacement's subscriptions on top of its own. Impact: Only the pending-resolve race is closed; normal resolves, same-view re-resolves, and dispose-then-revive via a new provider instance are unchanged. Both the sidebar (WebviewView) and tab (WebviewPanel) shapes share this code path, so both are covered by the single guard. --- src/core/webview/ClineProvider.ts | 9 ++++ .../webview/__tests__/ClineProvider.spec.ts | 49 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 2422b4c724..60311c028b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1126,6 +1126,15 @@ export class ClineProvider }, ) + // The awaits above yield; disposal or a view replacement may have + // landed while this resolve was pending. Installing listeners or the + // watchdog now would leak an interval on a disposed provider (the + // recovery path's _disposed check cannot stop the interval itself) or + // duplicate the replacement view's subscriptions. + if (this._disposed || this.view !== webviewView) { + return + } + // Sets up an event listener to listen for messages passed from the webview view context // and executes code based on the message that is received. this.setWebviewMessageListener(webviewView) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 44e7424e7b..cf8bcb2ab1 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1419,6 +1419,55 @@ describe("ClineProvider", () => { panelB.fireViewState() expect(provider["webviewHeartbeatRevision"]).toBe(revisionBefore + 1) }) + + test("installs no listeners or watchdog when the provider is disposed mid-resolve", async () => { + // Hold the resolve's initial HTML generation so disposal lands + // inside the pending resolve. + let finishHtml: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + finishHtml = resolve + }), + ) + + const resolvePromise = provider.resolveWebviewView(mockWebviewView) + await provider.dispose() + finishHtml("initial") + await resolvePromise + + expect(provider["webviewWatchdogInterval"]).toBeNull() + expect(provider["resolvedViewDisposables"].length).toBe(0) + }) + + test("installs nothing when a different view replaces the pending resolve", async () => { + // A's initial HTML stays pending; B's resolves immediately. + let finishHtmlA: (html: string) => void = () => {} + provider["getWebviewHtml"] = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishHtmlA = resolve + }), + ) + .mockImplementation(() => Promise.resolve("viewB-initial")) + + const resolveA = provider.resolveWebviewView(mockWebviewView) + const { viewB } = createViewB() + await provider.resolveWebviewView(viewB) + + finishHtmlA("stale-A") + await resolveA + + // A's stale resolve bailed out: only B's subscriptions (message, + // visibility, active editor, configuration) are installed and B + // keeps the watchdog. + expect(provider["resolvedViewDisposables"].length).toBe(4) + expect(provider["webviewWatchdogInterval"]).not.toBeNull() + // @ts-ignore - accessing private property for testing + expect(provider.view).toBe(viewB) + }) }) describe("tab panel (WebviewPanel shape)", () => { From 325436db84a4f12061f767ecd70cb2e8cead4ab2 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 29 Sep 2026 04:14:05 +0900 Subject: [PATCH 16/18] fix(webview): skip the initial html assignment for a stale resolve What: resolveWebviewView now checks _disposed and this.view === webviewView after the getWebviewHtml await and before assigning webview.html; a stale resolve returns without touching the view. The existing post-awaits guard before listener installation is unchanged. The two mid-resolve regression specs now also assert no html landed on the disposed/replaced view. Why: Disposal or replacement during html generation used to let the pending resolve assign html afterwards. The VS Code API throws when assigning to a destroyed webview, so the rejection surfaced from resolveWebviewView instead of returning cleanly, and a replaced resolve pushed html into the obsolete view it no longer owned. Impact: Live resolves assign exactly as before (a single combined guard after both awaits was rejected: it would leave the webview blank when the subsequent getState read fails, where today the html stays assigned). Same- view re-resolves, replacements, and revive flows are unaffected. --- src/core/webview/ClineProvider.ts | 10 +++++++++- src/core/webview/__tests__/ClineProvider.spec.ts | 9 ++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 60311c028b..fbd7901daf 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1094,7 +1094,15 @@ export class ClineProvider localResourceRoots: resourceRoots, } - webviewView.webview.html = await this.getWebviewHtml(webviewView.webview) + const html = await this.getWebviewHtml(webviewView.webview) + // The await yields; a disposal or replacement that landed mid-generation + // must not receive this HTML. The VS Code API throws when assigning to a + // destroyed webview, and a stale resolve must not touch the obsolete + // view it no longer owns. + if (this._disposed || this.view !== webviewView) { + return + } + webviewView.webview.html = html // Initialize out-of-scope variables that need to receive persistent // global state values. diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index cf8bcb2ab1..cd58da5493 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1436,6 +1436,8 @@ describe("ClineProvider", () => { finishHtml("initial") await resolvePromise + // The stale resolve assigned no HTML and installed nothing. + expect(mockWebviewView.webview.html).toBe("") expect(provider["webviewWatchdogInterval"]).toBeNull() expect(provider["resolvedViewDisposables"].length).toBe(0) }) @@ -1460,9 +1462,10 @@ describe("ClineProvider", () => { finishHtmlA("stale-A") await resolveA - // A's stale resolve bailed out: only B's subscriptions (message, - // visibility, active editor, configuration) are installed and B - // keeps the watchdog. + // A's stale resolve bailed out: no HTML landed on the obsolete + // view, only B's subscriptions (message, visibility, active + // editor, configuration) are installed and B keeps the watchdog. + expect(mockWebviewView.webview.html).toBe("") expect(provider["resolvedViewDisposables"].length).toBe(4) expect(provider["webviewWatchdogInterval"]).not.toBeNull() // @ts-ignore - accessing private property for testing From 63c82c3113ad1526e058d9aceeacbe266fa8cb4c Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 6 Oct 2026 04:13:51 +0900 Subject: [PATCH 17/18] chore: retrigger review-state reconciliation From 09855104bae12d41b17c62565a88dc00fba47747 Mon Sep 17 00:00:00 2001 From: myk1yt Date: Tue, 6 Oct 2026 05:29:10 +0900 Subject: [PATCH 18/18] chore: retrigger review-state reconciliation