-
Notifications
You must be signed in to change notification settings - Fork 268
fix(webview) searchFiles memory leak / WebUI Gray Screen #1360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1273db5
2bf97ed
f998638
cd16dba
1518739
188385d
6dfd8c6
7cc924c
1434e45
7f5db58
77dbbfa
e047340
478d28f
df78654
eee0c8d
3153327
9f96f10
55ed442
4a067c8
aa476fc
2a4e85a
8b49070
87d33d7
c4f3c18
8f3656c
7a533ca
fe61d99
a117660
3a957ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,12 @@ export interface ExtensionMessage { | |
| | "theme" | ||
| | "workspaceUpdated" | ||
| | "invoke" | ||
| | "messageUpdated" | ||
| | "clineMessageAppended" | ||
| | "clineMessageUpdated" | ||
| | "clineMessagesSnapshotStart" | ||
| | "clineMessagesSnapshotChunk" | ||
| | "clineMessagesSnapshotEnd" | ||
| | "messageUpdated" // Legacy: a patched webview requests a full resync instead of applying this. | ||
| | "mcpServers" | ||
| | "enhancedPrompt" | ||
| | "commitSearchResults" | ||
|
|
@@ -138,7 +143,13 @@ export interface ExtensionMessage { | |
| isActive: boolean | ||
| path?: string | ||
| }> | ||
| taskId?: string | ||
| clineMessage?: ClineMessage | ||
| clineMessages?: ClineMessage[] | ||
| clineMessagesSeq?: number | ||
| snapshotId?: string | ||
| snapshotStartIndex?: number | ||
| snapshotTotal?: number | ||
| routerModels?: RouterModels | ||
| openAiModels?: string[] | ||
| ollamaModels?: ModelRecord | ||
|
|
@@ -334,7 +345,11 @@ export type ExtensionState = Pick< | |
| lockApiConfigAcrossModes?: boolean | ||
| version: string | ||
| clineMessages: ClineMessage[] | ||
| currentTaskId?: string | ||
| /** | ||
| * Focused task identity. Omitted means this partial state update does not | ||
| * change task focus; null authoritatively means no task is focused. | ||
| */ | ||
| currentTaskId?: string | null | ||
| currentTaskItem?: HistoryItem | ||
| currentTaskTodos?: TodoItem[] // Initial todos for the current task | ||
| apiConfiguration: ProviderSettings | ||
|
|
@@ -426,10 +441,9 @@ export type ExtensionState = Pick< | |
| arch?: string | ||
|
|
||
| /** | ||
| * Monotonically increasing sequence number for clineMessages state pushes. | ||
| * When present, the frontend should only apply clineMessages from a state push | ||
| * if its seq is greater than the last applied seq. This prevents stale state | ||
| * (captured during async getStateToPostToWebview) from overwriting newer messages. | ||
| * Last sequence applied by the dedicated task-scoped transcript transport. | ||
| * Generic `state` messages intentionally omit this field and `clineMessages`; | ||
| * snapshots and append/update messages carry both transcript data and sequence. | ||
| */ | ||
| clineMessagesSeq?: number | ||
| } | ||
|
|
@@ -646,8 +660,11 @@ export interface WebviewMessage { | |
| | "openRuleFile" | ||
| | "openRulesDirectory" | ||
| | "themeFixtureProbeResponse" | ||
| | "requestClineMessagesResync" | ||
| text?: string | ||
| taskId?: string | ||
| expectedSeq?: number | ||
| receivedSeq?: number | ||
|
Comment on lines
665
to
+667
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which of |
||
| editedMessageContent?: string | ||
| tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "cloud" | ||
| disabled?: boolean | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -171,6 +171,7 @@ function queuedResponseForAsk(type: ClineAsk, text?: string): QueuedAskResolutio | |||||
|
|
||||||
| const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors | ||||||
| const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors | ||||||
| const PARTIAL_MESSAGE_UPDATE_DEBOUNCE_MS = 500 | ||||||
|
|
||||||
| export interface TaskOptions extends CreateTaskOptions { | ||||||
| provider: ClineProvider | ||||||
|
|
@@ -492,6 +493,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| // Token Usage Throttling - Debounced emit function | ||||||
| private readonly TOKEN_USAGE_EMIT_INTERVAL_MS = 2000 // 2 seconds | ||||||
| private debouncedEmitTokenUsage: ReturnType<typeof debounce> | ||||||
| private debouncedPostPartialMessageUpdate: ReturnType<typeof debounce> | ||||||
|
|
||||||
| // Historical cloud sync tracking retained only to avoid task resume churn. | ||||||
| private cloudSyncedMessageTimestamps: Set<number> = new Set() | ||||||
|
|
@@ -656,6 +658,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| this.TOKEN_USAGE_EMIT_INTERVAL_MS, | ||||||
| { leading: true, trailing: true, maxWait: this.TOKEN_USAGE_EMIT_INTERVAL_MS }, | ||||||
| ) | ||||||
| this.debouncedPostPartialMessageUpdate = debounce((message: ClineMessage) => { | ||||||
| const provider = this.providerRef.deref() | ||||||
| if (!provider) { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| void provider.postClineMessageUpdated(this.taskId, message).catch((error) => { | ||||||
| console.error("[Task#updateClineMessage] incremental post failed:", error) | ||||||
| }) | ||||||
| }, PARTIAL_MESSAGE_UPDATE_DEBOUNCE_MS) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The adjacent
Suggested change
|
||||||
|
|
||||||
| onCreated?.(this) | ||||||
|
|
||||||
|
|
@@ -1262,20 +1274,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| message.messageId ??= crypto.randomUUID() | ||||||
| this.clineMessages.push(message) | ||||||
| const provider = this.providerRef.deref() | ||||||
| // Unanswered asks must reach the webview before Message listeners can respond against its state. | ||||||
| const requiresImmediateState = | ||||||
| message.partial === true || (message.type === "ask" && message.isAnswered !== true) | ||||||
| try { | ||||||
| await provider?.postStateToWebviewThrottled() | ||||||
| await provider?.postClineMessageAppended(this.taskId, message) | ||||||
| } catch (error) { | ||||||
| console.error("[Task#addToClineMessages] postStateToWebviewThrottled failed:", error) | ||||||
| } | ||||||
| if (requiresImmediateState) { | ||||||
| try { | ||||||
| await provider?.flushPostStateToWebviewThrottled() | ||||||
| } catch (error) { | ||||||
| console.error("[Task#addToClineMessages] flushPostStateToWebviewThrottled failed:", error) | ||||||
| } | ||||||
| console.error("[Task#addToClineMessages] incremental post failed:", error) | ||||||
| } | ||||||
| this.emit(RooCodeEventName.Message, { action: "created", message }) | ||||||
| await this.saveClineMessages() | ||||||
|
|
@@ -1297,10 +1299,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| * Also resets cloud sync tracking to avoid re-syncing previously synced messages. | ||||||
| */ | ||||||
| public async overwriteClineMessages(newMessages: ClineMessage[], persist = true) { | ||||||
| this.debouncedPostPartialMessageUpdate.cancel() | ||||||
| this.hydrateClineMessages(newMessages) | ||||||
| if (persist) { | ||||||
| await this.saveClineMessages(false) | ||||||
| } | ||||||
| await this.providerRef.deref()?.postClineMessagesSnapshot(this.taskId, { bumpSeq: true }) | ||||||
|
Gh0st352 marked this conversation as resolved.
|
||||||
| } | ||||||
|
|
||||||
| private hydrateClineMessages(messages: ClineMessage[]) { | ||||||
|
|
@@ -1326,8 +1330,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| * Non-partial messages are synced to cloud telemetry if not already synced. | ||||||
| */ | ||||||
| private async updateClineMessage(message: ClineMessage) { | ||||||
| const provider = this.providerRef.deref() | ||||||
| await provider?.postMessageToWebview({ type: "messageUpdated", clineMessage: message }) | ||||||
| if (message.partial === true) { | ||||||
| this.debouncedPostPartialMessageUpdate(message) | ||||||
| } else { | ||||||
| this.debouncedPostPartialMessageUpdate.cancel() | ||||||
| await this.providerRef.deref()?.postClineMessageUpdated(this.taskId, message) | ||||||
| } | ||||||
| this.emit(RooCodeEventName.Message, { action: "updated", message }) | ||||||
|
Gh0st352 marked this conversation as resolved.
|
||||||
|
|
||||||
| // Check if we should sync to cloud and haven't already synced this message | ||||||
|
|
@@ -1422,7 +1430,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
|
|
||||||
| let askTs: number | ||||||
|
|
||||||
| // Resolve auto-approval before adding the message so the state snapshot | ||||||
| // Resolve auto-approval before adding the message so the incremental append | ||||||
| // sent to the webview already carries isAnswered:true when the ask will | ||||||
| // be immediately resolved. This eliminates the race between the state | ||||||
| // update (which shows approval buttons) and the former separate | ||||||
|
|
@@ -1456,10 +1464,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| lastMessage.partial = partial | ||||||
| lastMessage.progressStatus = progressStatus | ||||||
| lastMessage.isProtected = isProtected | ||||||
| // TODO: Be more efficient about saving and posting only new | ||||||
| // data or one whole message at a time so ignore partial for | ||||||
| // saves, and only post parts of partial message instead of | ||||||
| // whole array in new listener. | ||||||
| // Persist partial messages only when they become complete; the | ||||||
| // dedicated transport can still update one in-memory message at a time. | ||||||
| // Fire-and-forget: the webview post is internally guarded, but | ||||||
| // the `RooCodeEventName.Message` emit can synchronously throw | ||||||
| // if any consumer-attached listener does, which would surface | ||||||
|
|
@@ -1712,6 +1718,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| if (lastFollowUpIndex !== -1) { | ||||||
| // Mark this follow-up as answered | ||||||
| this.clineMessages[lastFollowUpIndex].isAnswered = true | ||||||
| void this.updateClineMessage(this.clineMessages[lastFollowUpIndex]).catch((error) => { | ||||||
| console.error("[Task#handleWebviewAskResponse] follow-up delta failed:", error) | ||||||
| }) | ||||||
| // Save the updated messages | ||||||
| this.saveClineMessages().catch((error) => { | ||||||
| console.error("Failed to save answered follow-up state:", error) | ||||||
|
|
@@ -2187,7 +2196,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| // The todo list is already set in the constructor if initialTodos were provided | ||||||
| // No need to add any messages - the todoList property is already set | ||||||
|
|
||||||
| await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() | ||||||
| await this.providerRef.deref()?.postClineMessagesSnapshot(this.taskId, { bumpSeq: true }) | ||||||
|
|
||||||
| await this.say("text", task, images) | ||||||
|
|
||||||
|
|
@@ -2332,16 +2341,23 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| await this.clearPendingActionAfterDurableResult(this.pendingAction.actionId) | ||||||
| } | ||||||
|
|
||||||
| if (this.pendingAction) { | ||||||
| this.isInitialized = true | ||||||
| await this.resumePendingTaskAction(this.pendingAction) | ||||||
| if (this.abort || this.abandoned) { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| // Publish the transcript after both histories hydrate, before any resume prompt or pending-action replay. | ||||||
| await this.providerRef.deref()?.postClineMessagesSnapshot(this.taskId, { bumpSeq: true }) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a test that pins the ordering of this snapshot relative to |
||||||
|
|
||||||
| if (this.abort || this.abandoned) { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if (this.pendingAction) { | ||||||
| this.isInitialized = true | ||||||
| await this.resumePendingTaskAction(this.pendingAction) | ||||||
| return | ||||||
| } | ||||||
|
Gh0st352 marked this conversation as resolved.
|
||||||
|
|
||||||
| const lastClineMessage = this.clineMessages | ||||||
| .slice() | ||||||
| .reverse() | ||||||
|
|
@@ -2356,7 +2372,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
|
|
||||||
| this.isInitialized = true | ||||||
|
|
||||||
| const { response, text, images } = await this.ask(askType) // Calls `postStateToWebview`. | ||||||
| const { response, text, images } = await this.ask(askType) | ||||||
|
|
||||||
| let responseText: string | undefined | ||||||
| let responseImages: string[] | undefined | ||||||
|
|
@@ -2687,6 +2703,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| private async disposeOnce(): Promise<void> { | ||||||
| console.log(`[Task#dispose] disposing task ${this.taskId}.${this.instanceId}`) | ||||||
| this.cancelAssistantMessagePersistence() | ||||||
| this.debouncedPostPartialMessageUpdate.cancel() | ||||||
|
|
||||||
| // Stop the idle telemetry check and report any unflushed activity as a | ||||||
| // shutdown installment, so a task torn down mid-work (panel closed, task | ||||||
|
|
@@ -3071,7 +3088,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| } satisfies ClineApiReqInfo) | ||||||
|
|
||||||
| await this.saveClineMessages() | ||||||
| await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() | ||||||
| const apiRequestMessage = this.clineMessages[lastApiReqIndex] | ||||||
| if (apiRequestMessage) { | ||||||
| await this.updateClineMessage(apiRequestMessage) | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| let cacheWriteTokens = 0 | ||||||
|
|
@@ -3142,12 +3162,16 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| if (lastMessage && lastMessage.partial) { | ||||||
| // lastMessage.ts = Date.now() DO NOT update ts since it is used as a key for virtuoso list | ||||||
| lastMessage.partial = false | ||||||
| // instead of streaming partialMessage events, we do a save and post like normal to persist to disk | ||||||
| await this.updateClineMessage(lastMessage) | ||||||
| } | ||||||
|
|
||||||
| // Update `api_req_started` to have cancelled and cost, so that | ||||||
| // we can display the cost of the partial stream and the cancellation reason | ||||||
| updateApiReqMsg(cancelReason, streamingFailedMessage) | ||||||
| const apiRequestMessage = this.clineMessages[lastApiReqIndex] | ||||||
| if (apiRequestMessage) { | ||||||
| await this.updateClineMessage(apiRequestMessage) | ||||||
| } | ||||||
| await this.saveClineMessages() | ||||||
|
|
||||||
| // Signals to provider that it can retrieve the saved messages | ||||||
|
|
@@ -3789,7 +3813,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||
| } | ||||||
|
|
||||||
| await this.saveClineMessages() | ||||||
| await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() | ||||||
|
|
||||||
| // No legacy text-stream tool parser state to reset. | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seven new transport fields (
taskId,clineMessage,clineMessages,clineMessagesSeq,snapshotId,snapshotStartIndex,snapshotTotal) have no JSDoc. The PR description mentions protocol docs embedded in JSDoc — are these intended to go here?