diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index da1d214cdbb..2e738629271 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -388,6 +388,14 @@ export type ExtensionState = Pick< featureRoomoteControlEnabled: boolean openAiCodexIsAuthenticated?: boolean debug?: boolean + + /** + * 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. + */ + clineMessagesSeq?: number } export interface Command { diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 0bdbb26d462..446a91f77c8 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -188,6 +188,7 @@ vi.mock("../core/webview/ClineProvider", async () => { resolveWebviewView: vi.fn(), postMessageToWebview: vi.fn(), postStateToWebview: vi.fn(), + postStateToWebviewWithoutClineMessages: vi.fn(), getState: vi.fn().mockResolvedValue({}), remoteControlEnabled: vi.fn().mockImplementation(async (enabled: boolean) => { if (!enabled) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index cef6ce94707..7c3556a2f2b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -159,6 +159,12 @@ export class ClineProvider private cloudOrganizationsCacheTimestamp: number | null = null private static readonly CLOUD_ORGANIZATIONS_CACHE_DURATION_MS = 5 * 1000 // 5 seconds + /** + * Monotonically increasing sequence number for clineMessages state pushes. + * Used by the frontend to reject stale state that arrives out-of-order. + */ + private clineMessagesSeq = 0 + public isViewLaunched = false public settingsImportedAt?: number public readonly latestAnnouncementId = "feb-2026-v3.47.0-opus-4.6-gpt-5.3-codex" // v3.47.0 Claude Opus 4.6 & GPT-5.3-Codex @@ -192,7 +198,7 @@ export class ClineProvider this.providerSettingsManager = new ProviderSettingsManager(this.context) this.customModesManager = new CustomModesManager(this.context, async () => { - await this.postStateToWebview() + await this.postStateToWebviewWithoutClineMessages() }) // Initialize MCP Hub through the singleton manager @@ -389,7 +395,7 @@ export class ClineProvider await this.activateProviderProfile({ name: profile.name }) } - await this.postStateToWebview() + await this.postStateToWebviewWithoutClineMessages() } } catch (error) { this.log(`Error syncing cloud profiles: ${error}`) @@ -1913,6 +1919,8 @@ export class ClineProvider async postStateToWebview() { const state = await this.getStateToPostToWebview() + this.clineMessagesSeq++ + state.clineMessagesSeq = this.clineMessagesSeq this.postMessageToWebview({ type: "state", state }) // Check MDM compliance and send user to account tab if not compliant @@ -1932,6 +1940,8 @@ export class ClineProvider */ async postStateToWebviewWithoutTaskHistory(): Promise { const state = await this.getStateToPostToWebview() + this.clineMessagesSeq++ + state.clineMessagesSeq = this.clineMessagesSeq const { taskHistory: _omit, ...rest } = state this.postMessageToWebview({ type: "state", state: rest }) @@ -1941,6 +1951,28 @@ export class ClineProvider } } + /** + * Like postStateToWebview but intentionally omits both clineMessages and taskHistory. + * + * Rationale: + * - Cloud event handlers (auth, settings, user-info) and mode changes trigger state pushes + * that have nothing to do with chat messages. Including clineMessages in these pushes + * creates race conditions where a stale snapshot of clineMessages (captured during async + * getStateToPostToWebview) overwrites newer messages the task has streamed in the meantime. + * - This method ensures cloud/mode events only push the state fields they actually affect + * (cloud auth, org settings, profiles, etc.) without interfering with task message streaming. + */ + async postStateToWebviewWithoutClineMessages(): Promise { + const state = await this.getStateToPostToWebview() + const { clineMessages: _omitMessages, taskHistory: _omitHistory, ...rest } = state + this.postMessageToWebview({ type: "state", state: rest }) + + // Preserve existing MDM redirect behavior + if (this.mdmService?.requiresCloudAuth() && !this.checkMdmCompliance()) { + await this.postMessageToWebview({ type: "action", action: "cloudButtonClicked" }) + } + } + /** * Fetches marketplace data on demand to avoid blocking main state updates */ diff --git a/src/extension.ts b/src/extension.ts index 262bf623378..75fff6328f3 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -195,7 +195,7 @@ export async function activate(context: vscode.ExtensionContext) { const provider = new ClineProvider(context, outputChannel, "sidebar", contextProxy, mdmService) // Initialize Roo Code Cloud service. - const postStateListener = () => ClineProvider.getVisibleInstance()?.postStateToWebview() + const postStateListener = () => ClineProvider.getVisibleInstance()?.postStateToWebviewWithoutClineMessages() authStateChangedHandler = async (data: { state: AuthState; previousState: AuthState }) => { postStateListener() diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 2378873f010..c0319648aa6 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -175,6 +175,21 @@ export const mergeExtensionState = (prevState: ExtensionState, newState: Partial const experiments = { ...prevExperiments, ...(newExperiments ?? {}) } const rest = { ...prevRest, ...newRest } + // Protect clineMessages from stale state pushes using sequence numbering. + // Multiple async event sources (cloud auth, settings, task streaming) can trigger + // concurrent state pushes. If a stale push arrives after a newer one, its clineMessages + // would overwrite the newer messages. The sequence number prevents this by only applying + // clineMessages when the incoming seq is strictly greater than the last applied seq. + if ( + newState.clineMessagesSeq !== undefined && + prevState.clineMessagesSeq !== undefined && + newState.clineMessagesSeq <= prevState.clineMessagesSeq && + newState.clineMessages !== undefined + ) { + rest.clineMessages = prevState.clineMessages + rest.clineMessagesSeq = prevState.clineMessagesSeq + } + // Note that we completely replace the previous apiConfiguration and customSupportPrompts objects // with new ones since the state that is broadcast is the entire objects so merging is not necessary. return { @@ -386,6 +401,14 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode newClineMessages[lastIndex] = clineMessage return { ...prevState, clineMessages: newClineMessages } } + // Log a warning if messageUpdated arrives for a timestamp not in the + // frontend's clineMessages. With the seq guard and cloud event isolation + // (layers 1+2), this should not happen under normal conditions. If it + // does, it signals a state synchronization issue worth investigating. + console.warn( + `[messageUpdated] Received update for unknown message ts=${clineMessage.ts}, dropping. ` + + `Frontend has ${prevState.clineMessages.length} messages.`, + ) return prevState }) break diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index bfc60f1ace4..41dabd7f0c2 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -4,6 +4,7 @@ import { type ProviderSettings, type ExperimentId, type ExtensionState, + type ClineMessage, DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, } from "@roo-code/types" @@ -253,4 +254,153 @@ describe("mergeExtensionState", () => { customTools: false, }) }) + + describe("clineMessagesSeq protection", () => { + const baseState: ExtensionState = { + version: "", + mcpEnabled: false, + clineMessages: [], + taskHistory: [], + shouldShowAnnouncement: false, + enableCheckpoints: true, + writeDelayMs: 1000, + mode: "default", + experiments: {} as Record, + customModes: [], + maxOpenTabsContext: 20, + maxWorkspaceFiles: 100, + apiConfiguration: {}, + telemetrySetting: "unset", + showRooIgnoredFiles: true, + enableSubfolderRules: false, + renderContext: "sidebar", + cloudUserInfo: null, + organizationAllowList: { allowAll: true, providers: {} }, + autoCondenseContext: true, + autoCondenseContextPercent: 100, + cloudIsAuthenticated: false, + sharingEnabled: false, + publicSharingEnabled: false, + profileThresholds: {}, + hasOpenedModeSelector: false, + maxImageFileSize: 5, + maxTotalImageSize: 20, + remoteControlEnabled: false, + taskSyncEnabled: false, + featureRoomoteControlEnabled: false, + isBrowserSessionActive: false, + checkpointTimeout: DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, + } + + const makeMessage = (ts: number, text: string): ClineMessage => + ({ ts, type: "say", say: "text", text }) as ClineMessage + + it("rejects stale clineMessages when seq is not newer", () => { + const newerMessages = [makeMessage(1, "hello"), makeMessage(2, "world")] + const staleMessages = [makeMessage(1, "hello")] + + const prevState: ExtensionState = { + ...baseState, + clineMessages: newerMessages, + clineMessagesSeq: 5, + } + + const result = mergeExtensionState(prevState, { + clineMessages: staleMessages, + clineMessagesSeq: 3, // stale seq + }) + + // Should keep the newer messages + expect(result.clineMessages).toBe(newerMessages) + expect(result.clineMessagesSeq).toBe(5) + }) + + it("rejects clineMessages when seq equals current (not strictly greater)", () => { + const currentMessages = [makeMessage(1, "hello"), makeMessage(2, "world")] + const sameSeqMessages = [makeMessage(1, "hello")] + + const prevState: ExtensionState = { + ...baseState, + clineMessages: currentMessages, + clineMessagesSeq: 5, + } + + const result = mergeExtensionState(prevState, { + clineMessages: sameSeqMessages, + clineMessagesSeq: 5, // same seq, not strictly greater + }) + + expect(result.clineMessages).toBe(currentMessages) + expect(result.clineMessagesSeq).toBe(5) + }) + + it("accepts clineMessages when seq is strictly greater", () => { + const oldMessages = [makeMessage(1, "hello")] + const newMessages = [makeMessage(1, "hello"), makeMessage(2, "world")] + + const prevState: ExtensionState = { + ...baseState, + clineMessages: oldMessages, + clineMessagesSeq: 3, + } + + const result = mergeExtensionState(prevState, { + clineMessages: newMessages, + clineMessagesSeq: 4, // newer seq + }) + + expect(result.clineMessages).toBe(newMessages) + expect(result.clineMessagesSeq).toBe(4) + }) + + it("preserves clineMessages when newState does not include them (cloud event path)", () => { + const existingMessages = [makeMessage(1, "hello"), makeMessage(2, "world")] + + const prevState: ExtensionState = { + ...baseState, + clineMessages: existingMessages, + clineMessagesSeq: 5, + } + + // Simulate a cloud event push that omits clineMessages and clineMessagesSeq + const result = mergeExtensionState(prevState, { + cloudIsAuthenticated: true, + }) + + expect(result.clineMessages).toBe(existingMessages) + expect(result.clineMessagesSeq).toBe(5) + }) + + it("applies clineMessages normally when neither state has seq (backward compat)", () => { + const oldMessages = [makeMessage(1, "hello")] + const newMessages = [makeMessage(1, "hello"), makeMessage(2, "world")] + + const prevState: ExtensionState = { + ...baseState, + clineMessages: oldMessages, + } + + const result = mergeExtensionState(prevState, { + clineMessages: newMessages, + }) + + expect(result.clineMessages).toBe(newMessages) + }) + + it("applies clineMessages when prevState has no seq but newState does (first push)", () => { + const prevState: ExtensionState = { + ...baseState, + clineMessages: [], + } + + const newMessages = [makeMessage(1, "hello")] + const result = mergeExtensionState(prevState, { + clineMessages: newMessages, + clineMessagesSeq: 1, + }) + + expect(result.clineMessages).toBe(newMessages) + expect(result.clineMessagesSeq).toBe(1) + }) + }) })