mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Co-authored-by: Sannidhya <sann@Sannidhyas-MacBook-Pro.local>
This commit is contained in:
parent
d4cf3211e7
commit
ff89965f59
6 changed files with 217 additions and 3 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
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<void> {
|
||||
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
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<ExperimentId, boolean>,
|
||||
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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue