Skip to content

Batch settings updates from the webview to the extension host #9165

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 12, 2025
Merged

Batch settings updates from the webview to the extension host #9165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7b50159
Batch settings updates from the webview to the extension host
cte Nov 11, 2025
e217942
Remove debugging
cte Nov 11, 2025
85a478c
More tweaks
cte Nov 11, 2025
e6f8e3c
Remove debugging
cte Nov 11, 2025
3f96731
fix: throw error instead of returning empty array in getOrganizationM…
roomote Nov 11, 2025
9804568
More progress
cte Nov 11, 2025
697c12b
fix: reduce organization cache duration from 5 minutes to 5 seconds
roomote Nov 12, 2025
cea5d7b
Merge branch 'main' into cte/batch-settings-updates
cte Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/cloud/src/WebAuthService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,13 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A
signal: AbortSignal.timeout(10000),
})

return clerkOrganizationMembershipsSchema.parse(await response.json()).response
if (response.ok) {
return clerkOrganizationMembershipsSchema.parse(await response.json()).response
}

const errorMessage = `Failed to get organization memberships: ${response.status} ${response.statusText}`
this.log(`[auth] ${errorMessage}`)
throw new Error(errorMessage)
}

private async getOrganizationMetadata(
Expand Down
18 changes: 17 additions & 1 deletion src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ export class ClineProvider
private pendingOperations: Map<string, PendingEditOperation> = new Map()
private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds

private cloudOrganizationsCache: CloudOrganizationMembership[] | null = null
private cloudOrganizationsCacheTimestamp: number | null = null
private static readonly CLOUD_ORGANIZATIONS_CACHE_DURATION_MS = 5 * 1000 // 5 seconds

public isViewLaunched = false
public settingsImportedAt?: number
public readonly latestAnnouncementId = "nov-2025-v3.30.0-pr-fixer" // v3.30.0 PR Fixer announcement
Expand Down Expand Up @@ -1919,7 +1923,19 @@ export class ClineProvider

try {
if (!CloudService.instance.isCloudAgent) {
cloudOrganizations = await CloudService.instance.getOrganizationMemberships()
const now = Date.now()

if (
this.cloudOrganizationsCache !== null &&
this.cloudOrganizationsCacheTimestamp !== null &&
now - this.cloudOrganizationsCacheTimestamp < ClineProvider.CLOUD_ORGANIZATIONS_CACHE_DURATION_MS
) {
cloudOrganizations = this.cloudOrganizationsCache!
} else {
cloudOrganizations = await CloudService.instance.getOrganizationMemberships()
this.cloudOrganizationsCache = cloudOrganizations
this.cloudOrganizationsCacheTimestamp = now
}
Comment on lines 1920 to +1938
Copy link
Contributor

@roomote roomote bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The organization cache may store empty arrays from failed API calls for 5 minutes. When getOrganizationMemberships() returns an empty array due to an error (line 710 in WebAuthService.ts), this code caches it as valid data. Users won't see their actual organizations until the cache expires, even if the API becomes available again.

Consider checking if the result is a legitimate empty array (user has no orgs) vs an error case before caching, or add cache invalidation on auth state changes.

Fix it with Roo Code or mention @roomote and request a fix.

Copy link
Collaborator Author

@cte cte Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roomote Please fix this by throwing in getOrganizationMemberships instead of returning an empty array.

Copy link
Contributor

@roomote roomote bot Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixaroo Clock   See task on Roo Cloud

Fixed the reported issue. All local checks passed.

View commit | Revert commit

}
} catch (error) {
// Ignore this error.
Expand Down
28 changes: 14 additions & 14 deletions src/core/webview/__tests__/ClineProvider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// npx vitest core/webview/__tests__/ClineProvider.spec.ts
// pnpm --filter roo-cline test core/webview/__tests__/ClineProvider.spec.ts

import Anthropic from "@anthropic-ai/sdk"
import * as vscode from "vscode"
Expand Down Expand Up @@ -786,7 +786,7 @@ describe("ClineProvider", () => {
await provider.resolveWebviewView(mockWebviewView)
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

await messageHandler({ type: "writeDelayMs", value: 2000 })
await messageHandler({ type: "updateSettings", updatedSettings: { writeDelayMs: 2000 } })

expect(updateGlobalStateSpy).toHaveBeenCalledWith("writeDelayMs", 2000)
expect(mockContext.globalState.update).toHaveBeenCalledWith("writeDelayMs", 2000)
Expand All @@ -800,24 +800,24 @@ describe("ClineProvider", () => {
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

// Simulate setting sound to enabled
await messageHandler({ type: "soundEnabled", bool: true })
await messageHandler({ type: "updateSettings", updatedSettings: { soundEnabled: true } })
expect(updateGlobalStateSpy).toHaveBeenCalledWith("soundEnabled", true)
expect(mockContext.globalState.update).toHaveBeenCalledWith("soundEnabled", true)
expect(mockPostMessage).toHaveBeenCalled()

// Simulate setting sound to disabled
await messageHandler({ type: "soundEnabled", bool: false })
await messageHandler({ type: "updateSettings", updatedSettings: { soundEnabled: false } })
expect(mockContext.globalState.update).toHaveBeenCalledWith("soundEnabled", false)
expect(mockPostMessage).toHaveBeenCalled()

// Simulate setting tts to enabled
await messageHandler({ type: "ttsEnabled", bool: true })
await messageHandler({ type: "updateSettings", updatedSettings: { ttsEnabled: true } })
expect(setTtsEnabled).toHaveBeenCalledWith(true)
expect(mockContext.globalState.update).toHaveBeenCalledWith("ttsEnabled", true)
expect(mockPostMessage).toHaveBeenCalled()

// Simulate setting tts to disabled
await messageHandler({ type: "ttsEnabled", bool: false })
await messageHandler({ type: "updateSettings", updatedSettings: { ttsEnabled: false } })
expect(setTtsEnabled).toHaveBeenCalledWith(false)
expect(mockContext.globalState.update).toHaveBeenCalledWith("ttsEnabled", false)
expect(mockPostMessage).toHaveBeenCalled()
Expand Down Expand Up @@ -856,7 +856,7 @@ describe("ClineProvider", () => {
test("handles autoCondenseContext message", async () => {
await provider.resolveWebviewView(mockWebviewView)
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]
await messageHandler({ type: "autoCondenseContext", bool: false })
await messageHandler({ type: "updateSettings", updatedSettings: { autoCondenseContext: false } })
expect(updateGlobalStateSpy).toHaveBeenCalledWith("autoCondenseContext", false)
expect(mockContext.globalState.update).toHaveBeenCalledWith("autoCondenseContext", false)
expect(mockPostMessage).toHaveBeenCalled()
Expand All @@ -876,7 +876,7 @@ describe("ClineProvider", () => {
await provider.resolveWebviewView(mockWebviewView)
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

await messageHandler({ type: "autoCondenseContextPercent", value: 75 })
await messageHandler({ type: "updateSettings", updatedSettings: { autoCondenseContextPercent: 75 } })

expect(updateGlobalStateSpy).toHaveBeenCalledWith("autoCondenseContextPercent", 75)
expect(mockContext.globalState.update).toHaveBeenCalledWith("autoCondenseContextPercent", 75)
Expand Down Expand Up @@ -984,7 +984,7 @@ describe("ClineProvider", () => {
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

// Test browserToolEnabled
await messageHandler({ type: "browserToolEnabled", bool: true })
await messageHandler({ type: "updateSettings", updatedSettings: { browserToolEnabled: true } })
expect(mockContext.globalState.update).toHaveBeenCalledWith("browserToolEnabled", true)
expect(mockPostMessage).toHaveBeenCalled()

Expand All @@ -1002,13 +1002,13 @@ describe("ClineProvider", () => {
expect((await provider.getState()).showRooIgnoredFiles).toBe(false)

// Test showRooIgnoredFiles with true
await messageHandler({ type: "showRooIgnoredFiles", bool: true })
await messageHandler({ type: "updateSettings", updatedSettings: { showRooIgnoredFiles: true } })
expect(mockContext.globalState.update).toHaveBeenCalledWith("showRooIgnoredFiles", true)
expect(mockPostMessage).toHaveBeenCalled()
expect((await provider.getState()).showRooIgnoredFiles).toBe(true)

// Test showRooIgnoredFiles with false
await messageHandler({ type: "showRooIgnoredFiles", bool: false })
await messageHandler({ type: "updateSettings", updatedSettings: { showRooIgnoredFiles: false } })
expect(mockContext.globalState.update).toHaveBeenCalledWith("showRooIgnoredFiles", false)
expect(mockPostMessage).toHaveBeenCalled()
expect((await provider.getState()).showRooIgnoredFiles).toBe(false)
Expand All @@ -1019,13 +1019,13 @@ describe("ClineProvider", () => {
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

// Test alwaysApproveResubmit
await messageHandler({ type: "alwaysApproveResubmit", bool: true })
await messageHandler({ type: "updateSettings", updatedSettings: { alwaysApproveResubmit: true } })
expect(updateGlobalStateSpy).toHaveBeenCalledWith("alwaysApproveResubmit", true)
expect(mockContext.globalState.update).toHaveBeenCalledWith("alwaysApproveResubmit", true)
expect(mockPostMessage).toHaveBeenCalled()

// Test requestDelaySeconds
await messageHandler({ type: "requestDelaySeconds", value: 10 })
await messageHandler({ type: "updateSettings", updatedSettings: { requestDelaySeconds: 10 } })
expect(mockContext.globalState.update).toHaveBeenCalledWith("requestDelaySeconds", 10)
expect(mockPostMessage).toHaveBeenCalled()
})
Expand Down Expand Up @@ -1092,7 +1092,7 @@ describe("ClineProvider", () => {
await provider.resolveWebviewView(mockWebviewView)
const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0]

await messageHandler({ type: "maxWorkspaceFiles", value: 300 })
await messageHandler({ type: "updateSettings", updatedSettings: { maxWorkspaceFiles: 300 } })

expect(updateGlobalStateSpy).toHaveBeenCalledWith("maxWorkspaceFiles", 300)
expect(mockContext.globalState.update).toHaveBeenCalledWith("maxWorkspaceFiles", 300)
Expand Down
12 changes: 6 additions & 6 deletions src/core/webview/__tests__/webviewMessageHandler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,8 @@ describe("webviewMessageHandler - mcpEnabled", () => {

it("delegates enable=true to McpHub and posts updated state", async () => {
await webviewMessageHandler(mockClineProvider, {
type: "mcpEnabled",
bool: true,
type: "updateSettings",
updatedSettings: { mcpEnabled: true },
})

expect((mockClineProvider as any).getMcpHub).toHaveBeenCalledTimes(1)
Expand All @@ -733,8 +733,8 @@ describe("webviewMessageHandler - mcpEnabled", () => {

it("delegates enable=false to McpHub and posts updated state", async () => {
await webviewMessageHandler(mockClineProvider, {
type: "mcpEnabled",
bool: false,
type: "updateSettings",
updatedSettings: { mcpEnabled: false },
})

expect((mockClineProvider as any).getMcpHub).toHaveBeenCalledTimes(1)
Expand All @@ -747,8 +747,8 @@ describe("webviewMessageHandler - mcpEnabled", () => {
;(mockClineProvider as any).getMcpHub = vi.fn().mockReturnValue(undefined)

await webviewMessageHandler(mockClineProvider, {
type: "mcpEnabled",
bool: true,
type: "updateSettings",
updatedSettings: { mcpEnabled: true },
})

expect((mockClineProvider as any).getMcpHub).toHaveBeenCalledTimes(1)
Expand Down
Loading