Skip to content

Commit 57e9032

Browse files
committed
fix(provider): isolate profile mutations from focused tasks
1 parent 38d7e23 commit 57e9032

5 files changed

Lines changed: 227 additions & 17 deletions

File tree

src/core/webview/ClineProvider.ts

Lines changed: 74 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,37 @@ export class ClineProvider
196196
private globalStateWriteThroughTimer: ReturnType<typeof setTimeout> | null = null
197197
private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds
198198
private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
199+
private providerProfileMutationQueue = Promise.resolve()
199200

200201
private runDelegationTransition<T>(parentTaskId: string, fn: () => Promise<T>): Promise<T> {
201202
this.delegationTransitionLocks ??= new Map()
202203
return runDelegationTransition(this.delegationTransitionLocks, parentTaskId, fn)
203204
}
205+
206+
private enqueueProviderProfileMutation<T>(fn: () => Promise<T>): Promise<T> {
207+
const run = this.providerProfileMutationQueue.then(fn, fn)
208+
const callerResult = this.withProviderProfileMutationTimeout(run)
209+
this.providerProfileMutationQueue = run.then(
210+
() => undefined,
211+
() => undefined,
212+
)
213+
return callerResult
214+
}
215+
216+
private withProviderProfileMutationTimeout<T>(operation: Promise<T>): Promise<T> {
217+
let timeoutId: ReturnType<typeof setTimeout> | undefined
218+
const timeout = new Promise<never>((_, reject) => {
219+
timeoutId = setTimeout(() => {
220+
reject(new Error("Provider profile mutation timed out"))
221+
}, ClineProvider.PENDING_OPERATION_TIMEOUT_MS)
222+
})
223+
224+
return Promise.race([operation, timeout]).finally(() => {
225+
if (timeoutId) {
226+
clearTimeout(timeoutId)
227+
}
228+
})
229+
}
204230
private readonly pendingEditOperations: PendingEditOperationStore
205231

206232
private cloudOrganizationsCache: CloudOrganizationMembership[] | null = null
@@ -1506,9 +1532,15 @@ export class ClineProvider
15061532
/**
15071533
* Handle switching to a new mode, including updating the associated API configuration
15081534
* @param newMode The mode to switch to
1535+
* @param targetTask The task whose in-memory mode should be updated. Defaults to the
1536+
* current task. Pass null to apply only global mode/profile effects for a pending child.
15091537
*/
1510-
public async handleModeSwitch(newMode: Mode) {
1511-
const task = this.getCurrentTask()
1538+
public async handleModeSwitch(newMode: Mode, targetTask: Task | null | undefined = this.getCurrentTask()) {
1539+
return this.enqueueProviderProfileMutation(() => this.handleModeSwitchUnlocked(newMode, targetTask))
1540+
}
1541+
1542+
private async handleModeSwitchUnlocked(newMode: Mode, targetTask: Task | null | undefined): Promise<void> {
1543+
const task = targetTask
15121544

15131545
if (task) {
15141546
TelemetryService.instance.captureModeSwitch(task.taskId, newMode)
@@ -1545,7 +1577,9 @@ export class ClineProvider
15451577
// If workspace lock is on, keep the current API config — don't load mode-specific config
15461578
const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false)
15471579
if (lockApiConfigAcrossModes) {
1548-
await this.postStateToWebview()
1580+
if (targetTask !== null) {
1581+
await this.postStateToWebview()
1582+
}
15491583
return
15501584
}
15511585

@@ -1571,7 +1605,10 @@ export class ClineProvider
15711605
const hasActualSettings = !!fullProfile.apiProvider
15721606

15731607
if (hasActualSettings) {
1574-
await this.activateProviderProfile({ name: profile.name })
1608+
await this.activateProviderProfileUnlocked(
1609+
{ name: profile.name },
1610+
targetTask === null ? { skipCurrentTaskRebuild: true } : undefined,
1611+
)
15751612
} else {
15761613
// The task will continue with the current/default configuration.
15771614
}
@@ -1591,7 +1628,9 @@ export class ClineProvider
15911628
}
15921629
}
15931630

1594-
await this.postStateToWebview()
1631+
if (targetTask !== null) {
1632+
await this.postStateToWebview()
1633+
}
15951634
}
15961635

15971636
// Provider Profile Management
@@ -1607,8 +1646,9 @@ export class ClineProvider
16071646
*/
16081647
private updateTaskApiHandlerIfNeeded(
16091648
providerSettings: ProviderSettings,
1610-
options: { forceRebuild?: boolean } = {},
1649+
options: { forceRebuild?: boolean; skipCurrentTaskRebuild?: boolean } = {},
16111650
): void {
1651+
if (options.skipCurrentTaskRebuild) return
16121652
const task = this.getCurrentTask()
16131653
if (!task) return
16141654

@@ -1724,7 +1764,11 @@ export class ClineProvider
17241764
await this.postStateToWebview()
17251765
}
17261766

1727-
private async persistStickyProviderProfileToCurrentTask(apiConfigName: string): Promise<void> {
1767+
private async persistStickyProviderProfileToCurrentTask(
1768+
apiConfigName: string,
1769+
options: { skipCurrentTaskRebuild?: boolean } = {},
1770+
): Promise<void> {
1771+
if (options.skipCurrentTaskRebuild) return
17281772
const task = this.getCurrentTask()
17291773
if (!task) {
17301774
return
@@ -1754,12 +1798,28 @@ export class ClineProvider
17541798

17551799
async activateProviderProfile(
17561800
args: { name: string } | { id: string },
1757-
options?: { persistModeConfig?: boolean; persistTaskHistory?: boolean },
1801+
options?: {
1802+
persistModeConfig?: boolean
1803+
persistTaskHistory?: boolean
1804+
skipCurrentTaskRebuild?: boolean
1805+
},
17581806
) {
1807+
return this.enqueueProviderProfileMutation(() => this.activateProviderProfileUnlocked(args, options))
1808+
}
1809+
1810+
private async activateProviderProfileUnlocked(
1811+
args: { name: string } | { id: string },
1812+
options?: {
1813+
persistModeConfig?: boolean
1814+
persistTaskHistory?: boolean
1815+
skipCurrentTaskRebuild?: boolean
1816+
},
1817+
): Promise<void> {
17591818
const { name, id, ...providerSettings } = await this.providerSettingsManager.activateProfile(args)
17601819

17611820
const persistModeConfig = options?.persistModeConfig ?? true
17621821
const persistTaskHistory = options?.persistTaskHistory ?? true
1822+
const skipCurrentTaskRebuild = options?.skipCurrentTaskRebuild ?? false
17631823

17641824
// See `upsertProviderProfile` for a description of what this is doing.
17651825
await Promise.all([
@@ -1775,17 +1835,19 @@ export class ClineProvider
17751835
}
17761836

17771837
// Change the provider for the current task.
1778-
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
1838+
this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true, skipCurrentTaskRebuild })
17791839

17801840
// Update the current task's sticky provider profile, unless this activation is
17811841
// being used purely as a non-persisting restoration (e.g., reopening a task from history).
17821842
if (persistTaskHistory) {
1783-
await this.persistStickyProviderProfileToCurrentTask(name)
1843+
await this.persistStickyProviderProfileToCurrentTask(name, { skipCurrentTaskRebuild })
17841844
}
17851845

1786-
await this.postStateToWebview()
1846+
if (!skipCurrentTaskRebuild) {
1847+
await this.postStateToWebview()
1848+
}
17871849

1788-
if (providerSettings.apiProvider) {
1850+
if (providerSettings.apiProvider && !skipCurrentTaskRebuild) {
17891851
this.emit(RooCodeEventName.ProviderProfileChanged, { name, provider: providerSettings.apiProvider })
17901852
}
17911853
}

src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
import * as vscode from "vscode"
44

55
import { TelemetryService } from "@roo-code/telemetry"
6-
import { getModelId } from "@roo-code/types"
6+
import { getModelId, RooCodeEventName } from "@roo-code/types"
77

88
import { ContextProxy } from "../../config/ContextProxy"
9+
import type { Mode } from "../../../shared/modes"
910
import { Task, TaskOptions } from "../../task/Task"
1011
import { ClineProvider } from "../ClineProvider"
1112

@@ -110,6 +111,7 @@ vi.mock("../../task/Task", () => ({
110111
overwriteApiConversationHistory: vi.fn(),
111112
taskId: options?.historyItem?.id || "test-task-id",
112113
emit: vi.fn(),
114+
setTaskApiConfigName: vi.fn(),
113115
updateApiConfiguration: vi.fn().mockImplementation(function (this: any, newConfig: any) {
114116
this.apiConfiguration = newConfig
115117
}),
@@ -235,6 +237,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
235237
{ name: "test-config", id: "test-id", apiProvider: "openrouter", modelId: "openai/gpt-4" },
236238
]),
237239
setModeConfig: vi.fn(),
240+
getModeConfigId: vi.fn().mockResolvedValue(undefined),
238241
activateProfile: vi.fn().mockResolvedValue({
239242
name: "test-config",
240243
id: "test-id",
@@ -410,6 +413,104 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
410413
})
411414

412415
describe("activateProviderProfile", () => {
416+
test("serializes provider profile mutations without interleaving", async () => {
417+
const events: string[] = []
418+
let resolveFirst!: () => void
419+
420+
provider["providerSettingsManager"].activateProfile = vi
421+
.fn()
422+
.mockImplementationOnce(async () => {
423+
events.push("first:start")
424+
await new Promise<void>((resolve) => {
425+
resolveFirst = resolve
426+
})
427+
events.push("first:end")
428+
return {
429+
name: "first-profile",
430+
id: "first-id",
431+
apiProvider: "openrouter",
432+
openRouterModelId: "openai/gpt-4",
433+
}
434+
})
435+
.mockImplementationOnce(async () => {
436+
events.push("second:start")
437+
return {
438+
name: "second-profile",
439+
id: "second-id",
440+
apiProvider: "openrouter",
441+
openRouterModelId: "openai/gpt-4.1-mini",
442+
}
443+
})
444+
445+
const first = provider.activateProviderProfile({ name: "first-profile" })
446+
const second = provider.activateProviderProfile({ name: "second-profile" })
447+
448+
await Promise.resolve()
449+
expect(events).toEqual(["first:start"])
450+
451+
resolveFirst()
452+
await first
453+
await second
454+
455+
expect(events).toEqual(["first:start", "first:end", "second:start"])
456+
})
457+
458+
test("provider profile mutation rejection does not poison later queued mutations", async () => {
459+
const firstError = new Error("first profile failed")
460+
461+
provider["providerSettingsManager"].activateProfile = vi
462+
.fn()
463+
.mockRejectedValueOnce(firstError)
464+
.mockResolvedValueOnce({
465+
name: "second-profile",
466+
id: "second-id",
467+
apiProvider: "openrouter",
468+
openRouterModelId: "openai/gpt-4.1-mini",
469+
})
470+
471+
await expect(provider.activateProviderProfile({ name: "first-profile" })).rejects.toThrow(firstError)
472+
await expect(provider.activateProviderProfile({ name: "second-profile" })).resolves.toBeUndefined()
473+
})
474+
475+
test("fan-out preparation leaves the focused task untouched", async () => {
476+
const mockTask = new Task({
477+
...defaultTaskOptions,
478+
apiConfiguration: {
479+
apiProvider: "openrouter",
480+
openRouterModelId: "openai/gpt-4",
481+
},
482+
})
483+
await provider.addClineToStack(mockTask)
484+
provider["providerSettingsManager"].getModeConfigId = vi.fn().mockResolvedValue("ask-id")
485+
provider["providerSettingsManager"].listConfig = vi
486+
.fn()
487+
.mockResolvedValue([{ name: "ask-profile", id: "ask-id", apiProvider: "openrouter" }])
488+
provider["providerSettingsManager"].getProfile = vi.fn().mockResolvedValue({
489+
name: "ask-profile",
490+
id: "ask-id",
491+
apiProvider: "openrouter",
492+
openRouterModelId: "openai/gpt-4.1-mini",
493+
})
494+
provider["providerSettingsManager"].activateProfile = vi.fn().mockResolvedValue({
495+
name: "ask-profile",
496+
id: "ask-id",
497+
apiProvider: "openrouter",
498+
openRouterModelId: "openai/gpt-4.1-mini",
499+
})
500+
const emitSpy = vi.spyOn(provider, "emit")
501+
const postStateSpy = vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined)
502+
503+
await provider.handleModeSwitch("ask" as Mode, null)
504+
505+
expect(mockTask.updateApiConfiguration).not.toHaveBeenCalled()
506+
expect(mockTask.setTaskApiConfigName).not.toHaveBeenCalled()
507+
expect(emitSpy).not.toHaveBeenCalledWith(
508+
RooCodeEventName.ProviderProfileChanged,
509+
expect.objectContaining({ name: "ask-profile" }),
510+
)
511+
expect(postStateSpy).not.toHaveBeenCalled()
512+
})
513+
413514
test("calls updateApiConfiguration when provider/model unchanged but settings differ (explicit profile switch)", async () => {
414515
const mockTask = new Task({
415516
...defaultTaskOptions,

src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -374,14 +374,15 @@ describe("ClineProvider - Lock API Config Across Modes", () => {
374374
apiProvider: "anthropic",
375375
})
376376

377-
const activateProviderProfileSpy = vi
378-
.spyOn(provider, "activateProviderProfile")
379-
.mockResolvedValue(undefined)
377+
const activateProfileSpy = vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({
378+
name: "architect-profile",
379+
apiProvider: "anthropic",
380+
})
380381

381382
await provider.handleModeSwitch("architect")
382383

383384
expect(getModeConfigIdSpy).toHaveBeenCalledWith("architect")
384-
expect(activateProviderProfileSpy).toHaveBeenCalledWith({ name: "architect-profile" })
385+
expect(activateProfileSpy).toHaveBeenCalledWith({ name: "architect-profile" })
385386
})
386387
})
387388
})
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { describe, expect, it, vi } from "vitest"
2+
import type * as vscode from "vscode"
3+
4+
import { API } from "../api"
5+
import type { ClineProvider } from "../../core/webview/ClineProvider"
6+
7+
vi.mock("@roo-code/ipc", () => ({
8+
IpcServer: class {},
9+
}))
10+
11+
describe("API - configuration", () => {
12+
it("persists every supplied mode API config mapping", async () => {
13+
const setValues = vi.fn().mockResolvedValue(undefined)
14+
const saveConfig = vi.fn().mockResolvedValue("default-id")
15+
const setModeConfig = vi.fn().mockResolvedValue(undefined)
16+
const postStateToWebview = vi.fn().mockResolvedValue(undefined)
17+
const provider = {
18+
context: {},
19+
on: vi.fn(),
20+
contextProxy: { setValues },
21+
providerSettingsManager: { saveConfig, setModeConfig },
22+
postStateToWebview,
23+
} as unknown as ClineProvider
24+
const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel
25+
const api = new API(outputChannel, provider)
26+
27+
await api.setConfiguration({
28+
currentApiConfigName: "default",
29+
modeApiConfigs: { code: "code-config", architect: "architect-config" },
30+
})
31+
32+
expect(saveConfig).toHaveBeenCalledWith("default", expect.objectContaining({ currentApiConfigName: "default" }))
33+
expect(setModeConfig).toHaveBeenCalledTimes(2)
34+
expect(setModeConfig).toHaveBeenCalledWith("code", "code-config")
35+
expect(setModeConfig).toHaveBeenCalledWith("architect", "architect-config")
36+
expect(postStateToWebview).toHaveBeenCalledOnce()
37+
})
38+
})

src/extension/api.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
import { IpcServer } from "@roo-code/ipc"
2424

2525
import { Package } from "../shared/package"
26+
import type { Mode } from "../shared/modes"
2627
import { ClineProvider } from "../core/webview/ClineProvider"
2728
import { Terminal } from "../integrations/terminal/Terminal"
2829
import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry"
@@ -505,6 +506,13 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
505506
public async setConfiguration(values: RooCodeSettings) {
506507
await this.sidebarProvider.contextProxy.setValues(values)
507508
await this.sidebarProvider.providerSettingsManager.saveConfig(values.currentApiConfigName || "default", values)
509+
if (values.modeApiConfigs) {
510+
await Promise.all(
511+
Object.entries(values.modeApiConfigs).map(([mode, configId]) =>
512+
this.sidebarProvider.providerSettingsManager.setModeConfig(mode as Mode, configId),
513+
),
514+
)
515+
}
508516
await this.sidebarProvider.postStateToWebview()
509517
}
510518

0 commit comments

Comments
 (0)