From 1f7e78ea2cb6bdbe781c7a2db78a9ffa093ea70c Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Fri, 10 Oct 2025 17:52:50 -0600 Subject: [PATCH 1/4] fix(webview): throttle state/message updates; drop hidden-panel deltas; throttle indexing updates to prevent grey screens during long tasks --- src/core/task/Task.ts | 41 ++++++++++++- src/core/webview/ClineProvider.ts | 95 ++++++++++++++++++++++++++----- 2 files changed, 121 insertions(+), 15 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 851df91e6c5..6cc94dcace5 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -277,6 +277,11 @@ export class Task extends EventEmitter implements TaskLike { public readonly messageQueueService: MessageQueueService private messageQueueStateChangedHandler: (() => void) | undefined + // Throttled chat row update batching to reduce webview message flood + private messageUpdateBuffer: Map = new Map() + private messageUpdateTimer?: NodeJS.Timeout + private readonly MESSAGE_UPDATE_THROTTLE_MS = 33 // ~30 FPS + // Streaming isWaitingForFirstChunk = false isStreaming = false @@ -644,9 +649,12 @@ export class Task extends EventEmitter implements TaskLike { private async updateClineMessage(message: ClineMessage) { const provider = this.providerRef.deref() - await provider?.postMessageToWebview({ type: "messageUpdated", clineMessage: message }) + + // Emit internal event immediately (used for token usage, etc) this.emit(RooCodeEventName.Message, { action: "updated", message }) + // Telemetry capture remains unchanged below + const shouldCaptureMessage = message.partial !== true && CloudService.isEnabled() if (shouldCaptureMessage) { @@ -655,6 +663,37 @@ export class Task extends EventEmitter implements TaskLike { properties: { taskId: this.taskId, message }, }) } + + // If provider is unavailable, or panel is hidden, skip UI delta updates. + // The UI will resync on next visibility or full state push. + if (!provider || (typeof (provider as any).isVisible === "function" && !(provider as any).isVisible())) { + return + } + + // Batch UI updates within a short window to avoid overwhelming the webview + const ts = (message as any)?.ts as number | undefined + if (typeof ts === "number") { + this.messageUpdateBuffer.set(ts, message) + } else { + // Fallback: no timestamp, just send immediately + await provider.postMessageToWebview({ type: "messageUpdated", clineMessage: message }) + return + } + + if (!this.messageUpdateTimer) { + this.messageUpdateTimer = setTimeout(async () => { + try { + const batch = Array.from(this.messageUpdateBuffer.values()) + this.messageUpdateBuffer.clear() + this.messageUpdateTimer = undefined + for (const m of batch) { + await provider.postMessageToWebview({ type: "messageUpdated", clineMessage: m }) + } + } catch (e) { + console.error("[Task#updateClineMessage] Failed to flush message updates:", e) + } + }, this.MESSAGE_UPDATE_THROTTLE_MS) + } } private async saveClineMessages() { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 91b86879668..8887e4363a6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -148,6 +148,17 @@ export class ClineProvider public readonly providerSettingsManager: ProviderSettingsManager public readonly customModesManager: CustomModesManager + // Throttling/backpressure for heavy webview messages + private stateFlushQueued = false + private stateQueueDirty = false + private stateFlushPromise?: Promise + private readonly STATE_THROTTLE_MS = 33 // ~30 FPS coalescing + + // Throttle noisy indexing status updates + private indexStatusThrottleTimer?: NodeJS.Timeout + private pendingIndexStatus?: IndexProgressUpdate + private readonly INDEX_STATUS_THROTTLE_MS = 100 + constructor( readonly context: vscode.ExtensionContext, private readonly outputChannel: vscode.OutputChannel, @@ -799,6 +810,8 @@ export class ClineProvider const viewStateDisposable = webviewView.onDidChangeViewState(() => { if (this.view?.visible) { this.postMessageToWebview({ type: "action", action: "didBecomeVisible" }) + // Push latest state when tab becomes visible to avoid stale UI without flooding while hidden + void this.postStateToWebview() } }) @@ -808,6 +821,8 @@ export class ClineProvider const visibilityDisposable = webviewView.onDidChangeVisibility(() => { if (this.view?.visible) { this.postMessageToWebview({ type: "action", action: "didBecomeVisible" }) + // Ensure UI sync after becoming visible + void this.postStateToWebview() } }) @@ -971,7 +986,16 @@ export class ClineProvider } public async postMessageToWebview(message: ExtensionMessage) { - await this.view?.webview.postMessage(message) + try { + // Reduce background chatter: drop chat row updates when panel isn't visible + if (message.type === "messageUpdated" && !this.isVisible()) { + return + } + if (!this.view) return + await this.view.webview.postMessage(message) + } catch (error) { + this.log(`[postMessageToWebview] failed: ${error instanceof Error ? error.message : String(error)}`) + } } private async getHMRHtmlContent(webview: vscode.Webview): Promise { @@ -1602,13 +1626,40 @@ export class ClineProvider } async postStateToWebview() { - const state = await this.getStateToPostToWebview() - this.postMessageToWebview({ type: "state", state }) + // Coalesce multiple rapid callers into a single flush, awaited by all + this.stateQueueDirty = true + if (!this.stateFlushQueued) { + this.stateFlushQueued = true + this.stateFlushPromise = this.flushStateQueue() + } + return this.stateFlushPromise + } + + private async flushStateQueue(): Promise { + try { + // Small delay window to coalesce bursts from multiple sources + await delay(this.STATE_THROTTLE_MS) + + // Drain while new calls arrive during awaits + do { + this.stateQueueDirty = false - // Check MDM compliance and send user to account tab if not compliant - // Only redirect if there's an actual MDM policy requiring authentication - if (this.mdmService?.requiresCloudAuth() && !this.checkMdmCompliance()) { - await this.postMessageToWebview({ type: "action", action: "cloudButtonClicked" }) + const state = await this.getStateToPostToWebview() + await this.postMessageToWebview({ type: "state", state }) + + // Check MDM compliance and send user to account tab if not compliant + // Only redirect if there's an actual MDM policy requiring authentication + if (this.mdmService?.requiresCloudAuth() && !this.checkMdmCompliance()) { + await this.postMessageToWebview({ type: "action", action: "cloudButtonClicked" }) + } + + // Allow any additional state requests queued during the await to be coalesced + if (this.stateQueueDirty) { + await delay(this.STATE_THROTTLE_MS) + } + } while (this.stateQueueDirty) + } finally { + this.stateFlushQueued = false } } @@ -2288,6 +2339,11 @@ export class ClineProvider return this._workspaceTracker } + // Public visibility helper for rate limiting background chatter + public isVisible(): boolean { + return this.view?.visible === true + } + get viewLaunched() { return this.isViewLaunched } @@ -2409,13 +2465,24 @@ export class ClineProvider if (currentManager) { this.codeIndexStatusSubscription = currentManager.onProgressUpdate((update: IndexProgressUpdate) => { // Only send updates if this manager is still the current one - if (currentManager === this.getCurrentWorkspaceCodeIndexManager()) { - // Get the full status from the manager to ensure we have all fields correctly formatted - const fullStatus = currentManager.getCurrentStatus() - this.postMessageToWebview({ - type: "indexingStatusUpdate", - values: fullStatus, - }) + if (currentManager !== this.getCurrentWorkspaceCodeIndexManager()) { + return + } + + // Throttle: coalesce frequent progress updates into ~10 Hz + this.pendingIndexStatus = currentManager.getCurrentStatus() + + if (!this.indexStatusThrottleTimer) { + this.indexStatusThrottleTimer = setTimeout(() => { + const values = this.pendingIndexStatus ?? currentManager.getCurrentStatus() + this.pendingIndexStatus = undefined + this.indexStatusThrottleTimer = undefined + + this.postMessageToWebview({ + type: "indexingStatusUpdate", + values, + }) + }, this.INDEX_STATUS_THROTTLE_MS) } }) From 03d28f90f127958a19a676c4ad9812495409a645 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Fri, 10 Oct 2025 18:33:10 -0600 Subject: [PATCH 2/4] =?UTF-8?q?fix(task):=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20move=20visibility=20check=20into=20flush,=20atomic?= =?UTF-8?q?=20batch=20swap,=20drop/log=20deltas=20when=20hidden?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/core/task/Task.ts | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 6cc94dcace5..85e4de55c19 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -654,9 +654,7 @@ export class Task extends EventEmitter implements TaskLike { this.emit(RooCodeEventName.Message, { action: "updated", message }) // Telemetry capture remains unchanged below - const shouldCaptureMessage = message.partial !== true && CloudService.isEnabled() - if (shouldCaptureMessage) { CloudService.instance.captureEvent({ event: TelemetryEventName.TASK_MESSAGE, @@ -664,18 +662,16 @@ export class Task extends EventEmitter implements TaskLike { }) } - // If provider is unavailable, or panel is hidden, skip UI delta updates. - // The UI will resync on next visibility or full state push. - if (!provider || (typeof (provider as any).isVisible === "function" && !(provider as any).isVisible())) { - return - } - // Batch UI updates within a short window to avoid overwhelming the webview const ts = (message as any)?.ts as number | undefined if (typeof ts === "number") { this.messageUpdateBuffer.set(ts, message) } else { - // Fallback: no timestamp, just send immediately + // Fallback: no timestamp - only send if panel is visible and provider exists + if (!provider || !provider.isVisible()) { + // Drop non-essential delta; UI will resync on next visibility/state push + return + } await provider.postMessageToWebview({ type: "messageUpdated", clineMessage: message }) return } @@ -683,11 +679,30 @@ export class Task extends EventEmitter implements TaskLike { if (!this.messageUpdateTimer) { this.messageUpdateTimer = setTimeout(async () => { try { - const batch = Array.from(this.messageUpdateBuffer.values()) - this.messageUpdateBuffer.clear() + // Atomically swap buffers so new arrivals during flush aren't lost + const batchMap = this.messageUpdateBuffer + this.messageUpdateBuffer = new Map() this.messageUpdateTimer = undefined + + const batch = Array.from(batchMap.values()) + + const providerNow = this.providerRef.deref() + if (!providerNow) { + console.warn( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas: provider unavailable`, + ) + return + } + if (!providerNow.isVisible()) { + // Drop deltas while hidden; UI will receive a full state sync on visibility + console.debug( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas while hidden`, + ) + return + } + for (const m of batch) { - await provider.postMessageToWebview({ type: "messageUpdated", clineMessage: m }) + await providerNow.postMessageToWebview({ type: "messageUpdated", clineMessage: m }) } } catch (e) { console.error("[Task#updateClineMessage] Failed to flush message updates:", e) From 458aac2eb56aa47d41af6779c71fe65f441b4188 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Sat, 11 Oct 2025 18:30:16 -0600 Subject: [PATCH 3/4] fix(webview): deterministic chat update ordering and safer throttles Task: sort batched messageUpdated by ts; clear messageUpdateTimer in dispose; use provider.log; remove any-cast for ts ClineProvider: void postMessage in throttle callbacks; clear indexStatusThrottleTimer in dispose --- src/core/task/Task.ts | 35 +++++++++++++++++++++++-------- src/core/webview/ClineProvider.ts | 11 ++++++++-- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 85e4de55c19..3bd97298315 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -663,7 +663,7 @@ export class Task extends EventEmitter implements TaskLike { } // Batch UI updates within a short window to avoid overwhelming the webview - const ts = (message as any)?.ts as number | undefined + const ts = message.ts as number | undefined if (typeof ts === "number") { this.messageUpdateBuffer.set(ts, message) } else { @@ -684,20 +684,24 @@ export class Task extends EventEmitter implements TaskLike { this.messageUpdateBuffer = new Map() this.messageUpdateTimer = undefined - const batch = Array.from(batchMap.values()) + const batch = Array.from(batchMap.values()).sort((a, b) => (a.ts ?? 0) - (b.ts ?? 0)) const providerNow = this.providerRef.deref() if (!providerNow) { - console.warn( - `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas: provider unavailable`, - ) + this.providerRef + .deref() + ?.log( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas: provider unavailable`, + ) return } if (!providerNow.isVisible()) { // Drop deltas while hidden; UI will receive a full state sync on visibility - console.debug( - `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas while hidden`, - ) + this.providerRef + .deref() + ?.log( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas while hidden`, + ) return } @@ -705,7 +709,13 @@ export class Task extends EventEmitter implements TaskLike { await providerNow.postMessageToWebview({ type: "messageUpdated", clineMessage: m }) } } catch (e) { - console.error("[Task#updateClineMessage] Failed to flush message updates:", e) + this.providerRef + .deref() + ?.log( + `[Task#updateClineMessage] Failed to flush message updates: ${ + e instanceof Error ? e.message : String(e) + }`, + ) } }, this.MESSAGE_UPDATE_THROTTLE_MS) } @@ -1581,6 +1591,13 @@ export class Task extends EventEmitter implements TaskLike { public dispose(): void { console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) + // Clear pending batched message timer and buffer + if (this.messageUpdateTimer) { + clearTimeout(this.messageUpdateTimer) + this.messageUpdateTimer = undefined + } + this.messageUpdateBuffer.clear() + // Dispose message queue and remove event listeners. try { if (this.messageQueueStateChangedHandler) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8887e4363a6..d5ff1943e35 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -583,6 +583,13 @@ export class ClineProvider async dispose() { this.log("Disposing ClineProvider...") + // Clear throttled timers + if (this.indexStatusThrottleTimer) { + clearTimeout(this.indexStatusThrottleTimer) + this.indexStatusThrottleTimer = undefined + } + this.pendingIndexStatus = undefined + // Clear all tasks from the stack. while (this.clineStack.length > 0) { await this.removeClineFromStack() @@ -2478,7 +2485,7 @@ export class ClineProvider this.pendingIndexStatus = undefined this.indexStatusThrottleTimer = undefined - this.postMessageToWebview({ + void this.postMessageToWebview({ type: "indexingStatusUpdate", values, }) @@ -2491,7 +2498,7 @@ export class ClineProvider } // Send initial status for the current workspace - this.postMessageToWebview({ + void this.postMessageToWebview({ type: "indexingStatusUpdate", values: currentManager.getCurrentStatus(), }) From 62557dcc0640a091084d8103ca0dfedeead0dd2d Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Sun, 12 Oct 2025 01:15:33 -0600 Subject: [PATCH 4/4] test: add vscode Uri/RelativePattern mocks and fix extension.spec; fix: guard dev FS watchers in activate() and MCP watchers in tests; core(task): safe provider logging in batched webview updates --- src/__mocks__/vscode.js | 4 +++ src/__tests__/extension.spec.ts | 8 +++++ src/core/task/Task.ts | 53 ++++++++++++++++++--------------- src/extension.ts | 7 ++++- src/services/mcp/McpHub.ts | 24 +++++++++++---- 5 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/__mocks__/vscode.js b/src/__mocks__/vscode.js index 7fc82f559f0..1715e49ca12 100644 --- a/src/__mocks__/vscode.js +++ b/src/__mocks__/vscode.js @@ -36,6 +36,8 @@ const mockSelection = class extends mockRange { } } +const mockRelativePattern = (base, pattern) => ({ base, pattern }) + export const workspace = { workspaceFolders: [], getWorkspaceFolder: () => null, @@ -107,6 +109,7 @@ export const env = { } export const Uri = mockUri +export const RelativePattern = mockRelativePattern export const Range = mockRange export const Position = mockPosition export const Selection = mockSelection @@ -160,6 +163,7 @@ export default { extensions, env, Uri, + RelativePattern, Range, Position, Selection, diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index c39854f697c..a4827157b98 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -40,6 +40,14 @@ vi.mock("vscode", () => ({ ExtensionMode: { Production: 1, }, + // Provide minimal implementations required by extension.ts watchers in dev + Uri: { + file: vi.fn().mockImplementation((p: string) => ({ fsPath: p })), + }, + RelativePattern: vi.fn().mockImplementation((base: { fsPath?: string } | string, pattern: string) => ({ + base: typeof base === "string" ? base : (base?.fsPath ?? ""), + pattern, + })), })) vi.mock("@dotenvx/dotenvx", () => ({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 3bd97298315..9f8ccb54626 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -303,6 +303,17 @@ export class Task extends EventEmitter implements TaskLike { private tokenUsageSnapshot?: TokenUsage private tokenUsageSnapshotAt?: number + private safeProviderLog(message: string): void { + try { + const provider = this.providerRef.deref() + if (provider && typeof (provider as any).log === "function") { + ;(provider as any).log(message) + } + } catch { + // no-op (tests/mocks may not implement log) + } + } + constructor({ provider, apiConfiguration, @@ -466,7 +477,7 @@ export class Task extends EventEmitter implements TaskLike { this._taskMode = defaultModeSlug // Use the provider's log method for better error visibility const errorMessage = `Failed to initialize task mode: ${error instanceof Error ? error.message : String(error)}` - provider.log(errorMessage) + this.safeProviderLog(errorMessage) } } @@ -688,20 +699,16 @@ export class Task extends EventEmitter implements TaskLike { const providerNow = this.providerRef.deref() if (!providerNow) { - this.providerRef - .deref() - ?.log( - `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas: provider unavailable`, - ) + this.safeProviderLog( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas: provider unavailable`, + ) return } if (!providerNow.isVisible()) { // Drop deltas while hidden; UI will receive a full state sync on visibility - this.providerRef - .deref() - ?.log( - `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas while hidden`, - ) + this.safeProviderLog( + `[Task#updateClineMessage] Dropping ${batch.length} messageUpdated deltas while hidden`, + ) return } @@ -709,13 +716,11 @@ export class Task extends EventEmitter implements TaskLike { await providerNow.postMessageToWebview({ type: "messageUpdated", clineMessage: m }) } } catch (e) { - this.providerRef - .deref() - ?.log( - `[Task#updateClineMessage] Failed to flush message updates: ${ - e instanceof Error ? e.message : String(e) - }`, - ) + this.safeProviderLog( + `[Task#updateClineMessage] Failed to flush message updates: ${ + e instanceof Error ? e.message : String(e) + }`, + ) } }, this.MESSAGE_UPDATE_THROTTLE_MS) } @@ -1741,9 +1746,9 @@ export class Task extends EventEmitter implements TaskLike { // including the subtask result, not just from before the subtask was created this.skipPrevResponseIdOnce = true } catch (error) { - this.providerRef - .deref() - ?.log(`Error failed to add reply from subtask into conversation of parent task, error: ${error}`) + this.safeProviderLog( + `Error failed to add reply from subtask into conversation of parent task, error: ${error}`, + ) throw error } @@ -1834,9 +1839,9 @@ export class Task extends EventEmitter implements TaskLike { const provider = this.providerRef.deref() if (this.isPaused && provider) { - provider.log(`[subtasks] paused ${this.taskId}.${this.instanceId}`) + this.safeProviderLog(`[subtasks] paused ${this.taskId}.${this.instanceId}`) await this.waitForSubtask() - provider.log(`[subtasks] resumed ${this.taskId}.${this.instanceId}`) + this.safeProviderLog(`[subtasks] resumed ${this.taskId}.${this.instanceId}`) const currentMode = (await provider.getState())?.mode ?? defaultModeSlug if (currentMode !== this.pausedModeSlug) { @@ -1846,7 +1851,7 @@ export class Task extends EventEmitter implements TaskLike { // Delay to allow mode change to take effect before next tool is executed. await delay(500) - provider.log( + this.safeProviderLog( `[subtasks] task ${this.taskId}.${this.instanceId} has switched back to '${this.pausedModeSlug}' from '${currentMode}'`, ) } diff --git a/src/extension.ts b/src/extension.ts index 5db0996ad65..5ebfa23297a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -273,7 +273,12 @@ export async function activate(context: vscode.ExtensionContext) { const enableLogging = typeof socketPath === "string" // Watch the core files and automatically reload the extension host. - if (process.env.NODE_ENV === "development") { + if ( + process.env.NODE_ENV === "development" && + (vscode as any).RelativePattern && + (vscode as any).Uri?.file && + vscode.workspace?.createFileSystemWatcher + ) { const watchPaths = [ { path: context.extensionPath, pattern: "**/*.ts" }, { path: path.join(context.extensionPath, "../packages/types"), pattern: "**/*.ts" }, diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index caca5ddb392..96daf1dd399 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -152,6 +152,10 @@ export class McpHub { private refCount: number = 0 // Reference counter for active clients private configChangeDebounceTimers: Map = new Map() + private isTestEnvironment(): boolean { + return process.env.NODE_ENV === "test" || !!process.env.VITEST || process.env.JEST === "true" + } + constructor(provider: ClineProvider) { this.providerRef = new WeakRef(provider) this.watchMcpSettingsFile() @@ -261,8 +265,7 @@ export class McpHub { } public setupWorkspaceFoldersWatcher(): void { - // Skip if test environment is detected - if (process.env.NODE_ENV === "test") { + if (this.isTestEnvironment() || !vscode.workspace?.onDidChangeWorkspaceFolders) { return } @@ -334,8 +337,12 @@ export class McpHub { } private async watchProjectMcpFile(): Promise { - // Skip if test environment is detected or VSCode APIs are not available - if (process.env.NODE_ENV === "test" || !vscode.workspace.createFileSystemWatcher) { + // Skip in test environments or when required VSCode APIs are unavailable + if ( + this.isTestEnvironment() || + !vscode.workspace?.createFileSystemWatcher || + !(vscode as any).RelativePattern + ) { return } @@ -467,8 +474,13 @@ export class McpHub { } private async watchMcpSettingsFile(): Promise { - // Skip if test environment is detected or VSCode APIs are not available - if (process.env.NODE_ENV === "test" || !vscode.workspace.createFileSystemWatcher) { + // Skip in test environments or when required VSCode APIs are unavailable + if ( + this.isTestEnvironment() || + !vscode.workspace?.createFileSystemWatcher || + !(vscode as any).Uri?.file || + !(vscode as any).RelativePattern + ) { return }