From ce0fb202a742c55cf062766680289adb8ea139ca Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Tue, 20 Oct 2026 21:41:11 +0200 Subject: [PATCH 1/9] fix(profiles): keep mode profile choices per window The mode -> provider profile map lives in shared extension storage, so a profile selected in one VS Code window leaked into subtasks and mode switches in other windows. Prefer this window's own choices and fall back to the shared map. Fixes #2109 Co-Authored-By: Claude Sonnet 5.5 --- src/core/webview/ClineProvider.ts | 28 +++++++++++++++---- .../webview/__tests__/ClineProvider.spec.ts | 15 ++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 28a42913f3..60806b9acd 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -357,6 +357,13 @@ export class ClineProvider public readonly providerSettingsManager: ProviderSettingsManager public readonly customModesManager: CustomModesManager + /** + * Mode -> profile id choices made by this window. The persisted mode map in + * `providerSettingsManager` is shared by every VS Code window, so reading it + * alone lets another window's profile selection leak into this window's tasks. + */ + private readonly localModeConfigIds = new Map() + constructor( readonly context: vscode.ExtensionContext, private readonly outputChannel: vscode.OutputChannel, @@ -1273,7 +1280,7 @@ export class ClineProvider const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false) if (!historyItem.apiConfigName && !lockApiConfigAcrossModes && !skipProfileRestoreFromHistory) { - const savedConfigId = await this.providerSettingsManager.getModeConfigId(historyItem.mode) + const savedConfigId = await this.getModeConfigId(historyItem.mode) const listApiConfig = await this.providerSettingsManager.listConfig() // Update listApiConfigMeta first to ensure UI has latest data. @@ -1775,7 +1782,7 @@ export class ClineProvider if (signal?.aborted) return // Load the saved API config for the new mode if it exists. - const savedConfigId = await this.providerSettingsManager.getModeConfigId(newMode) + const savedConfigId = await this.getModeConfigId(newMode) const listApiConfig = await this.providerSettingsManager.listConfig() if (signal?.aborted) return @@ -1817,7 +1824,7 @@ export class ClineProvider const config = listApiConfig.find((c) => c.name === currentApiConfigNameAfter) if (config?.id) { - await this.providerSettingsManager.setModeConfig(newMode, config.id) + await this.setModeConfig(newMode, config.id) } } } @@ -1829,6 +1836,15 @@ export class ClineProvider // Provider Profile Management + private async getModeConfigId(mode: Mode): Promise { + return this.localModeConfigIds.get(mode) ?? (await this.providerSettingsManager.getModeConfigId(mode)) + } + + private async setModeConfig(mode: Mode, id: string): Promise { + this.localModeConfigIds.set(mode, id) + await this.providerSettingsManager.setModeConfig(mode, id) + } + /** * Updates the current task's API handler. * Rebuilds when: @@ -2034,7 +2050,7 @@ export class ClineProvider const activationResults = await Promise.allSettled([ this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig()), this.updateGlobalState("currentApiConfigName", name), - this.providerSettingsManager.setModeConfig(mode, id), + this.setModeConfig(mode, id), this.contextProxy.setProviderSettings(providerSettings), ]) const failedActivation = activationResults.find( @@ -2156,7 +2172,7 @@ export class ClineProvider const { mode } = await this.getState() if (id && persistModeConfig) { - await this.providerSettingsManager.setModeConfig(mode, id) + await this.setModeConfig(mode, id) } // Change the provider for the current task. @@ -4007,7 +4023,7 @@ export class ClineProvider mode !== parentMode && this.context.workspaceState.get("lockApiConfigAcrossModes", false) let savedModeProfile: { name?: string; apiConfiguration: ProviderSettings } | undefined if (mode !== parentMode && !lockApiConfigAcrossModes) { - const savedConfigId = await this.providerSettingsManager.getModeConfigId(mode as Mode) + const savedConfigId = await this.getModeConfigId(mode as Mode) if (savedConfigId) { try { const { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 93543435e2..4da68d7904 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -2447,6 +2447,21 @@ describe("ClineProvider", () => { expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") }) + it("prefers this window's mode profile over one another window saved in shared storage", async () => { + await provider.resolveWebviewView(mockWebviewView) + + const manager = provider.providerSettingsManager + vi.spyOn(manager, "getModeConfigId").mockResolvedValue("other-window-id") + vi.spyOn(manager, "setModeConfig").mockResolvedValue(undefined) + + await provider["setModeConfig"]("code", "this-window-id") + + expect(manager.setModeConfig).toHaveBeenCalledWith("code", "this-window-id") + await expect(provider["getModeConfigId"]("code")).resolves.toBe("this-window-id") + // A mode this window never chose still falls back to the shared value. + await expect(provider["getModeConfigId"]("architect")).resolves.toBe("other-window-id") + }) + it("saves current config when switching to mode without config", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] From 2724c8fef1557e9dfd877a8e8cc0edbb49e6847f Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Tue, 20 Oct 2026 21:56:29 +0200 Subject: [PATCH 2/9] test: give delegation test doubles the window-local mode lookup Co-Authored-By: Claude Sonnet 5.5 --- src/__tests__/ClineProvider.delegation.spec.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/__tests__/ClineProvider.delegation.spec.ts b/src/__tests__/ClineProvider.delegation.spec.ts index 68f39c6517..fb5a5532b0 100644 --- a/src/__tests__/ClineProvider.delegation.spec.ts +++ b/src/__tests__/ClineProvider.delegation.spec.ts @@ -295,6 +295,8 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { isViewLaunched: false, taskHistoryStore: makeStoreStub(), providerSettingsManager, + // No window-local choice is recorded, so the lookup reaches the shared manager. + getModeConfigId: (mode: string) => providerSettingsManager.getModeConfigId(mode), context: { workspaceState: { get: workspaceGet } }, } as unknown as ClineProvider @@ -347,6 +349,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { ? vi.fn().mockRejectedValue(savedProfile) : vi.fn().mockResolvedValue(savedProfile) const log = vi.fn() + const getModeConfigId = vi.fn().mockResolvedValue(savedConfigId) const provider = { taskScheduler: new TaskScheduler(), emit: vi.fn(), @@ -356,10 +359,9 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { log, isViewLaunched: false, taskHistoryStore: makeStoreStub(), - providerSettingsManager: { - getModeConfigId: vi.fn().mockResolvedValue(savedConfigId), - getProfile, - }, + providerSettingsManager: { getModeConfigId, getProfile }, + // No window-local choice is recorded, so the lookup reaches the shared manager. + getModeConfigId, context: { workspaceState: { get: vi.fn().mockReturnValue(false) } }, } as unknown as ClineProvider From a4e06d411ae755671d951911d07298d1991668fa Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 25 Oct 2026 11:09:10 +0100 Subject: [PATCH 3/9] fix(webview): snapshot mode profile choices per window and clear them on reset Co-Authored-By: Claude Sonnet 5.5 --- .../ClineProvider.delegation.spec.ts | 47 +++++++++++++++++ src/core/config/ProviderSettingsManager.ts | 15 ++++++ .../__tests__/ProviderSettingsManager.spec.ts | 35 +++++++++++++ src/core/webview/ClineProvider.ts | 25 +++++++++- .../ClineProvider.apiHandlerRebuild.spec.ts | 2 + .../ClineProvider.lockApiConfig.spec.ts | 2 + .../webview/__tests__/ClineProvider.spec.ts | 50 +++++++++++++++++++ 7 files changed, 175 insertions(+), 1 deletion(-) diff --git a/src/__tests__/ClineProvider.delegation.spec.ts b/src/__tests__/ClineProvider.delegation.spec.ts index fb5a5532b0..7e8fc604fe 100644 --- a/src/__tests__/ClineProvider.delegation.spec.ts +++ b/src/__tests__/ClineProvider.delegation.spec.ts @@ -328,6 +328,53 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { ) }) + it("uses this window's mode profile instead of one another window saved in shared storage", async () => { + const parentTask = makeParentTask() + const createTask = vi + .fn() + .mockResolvedValue({ taskId: "child-local", run: vi.fn().mockResolvedValue(undefined) }) + const getProfile = vi.fn().mockResolvedValue({ + name: "local-profile", + id: "local-profile-id", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4.1-mini", + }) + const sharedGetModeConfigId = vi.fn().mockResolvedValue("other-window-profile-id") + const provider = { + taskScheduler: new TaskScheduler(), + emit: vi.fn(), + getCurrentTask: vi.fn(() => parentTask), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTask, + log: vi.fn(), + isViewLaunched: false, + taskHistoryStore: makeStoreStub(), + providerSettingsManager: { getModeConfigId: sharedGetModeConfigId, getProfile }, + // This window's own choice differs from the shared value another window wrote. + getModeConfigId: vi.fn().mockResolvedValue("local-profile-id"), + context: { workspaceState: { get: vi.fn().mockReturnValue(false) } }, + } as unknown as ClineProvider + + await ClineProvider.prototype.delegateParentAndOpenChild.call(provider, { + parentTaskId: "parent-1", + message: "Local child", + initialTodos: [], + mode: "ask", + }) + + expect(getProfile).toHaveBeenCalledOnce() + expect(getProfile).toHaveBeenCalledWith({ id: "local-profile-id" }) + expect(sharedGetModeConfigId).not.toHaveBeenCalled() + expect(createTask).toHaveBeenCalledWith( + "Local child", + undefined, + parentTask, + expect.objectContaining({ + handoffExecutionContext: expect.objectContaining({ mode: "ask", apiConfigName: "local-profile" }), + }), + ) + }) + it.each([ { name: "has no saved mode profile", savedConfigId: undefined, savedProfile: undefined }, { diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 01695d7994..a355b2ff01 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -574,6 +574,21 @@ export class ProviderSettingsManager { } } + /** + * Get a copy of the persisted mode -> API config ID map. The map is shared by every VS Code + * window, so callers that need a stable view must keep their own copy. + */ + public async getModeConfigIds(): Promise> { + try { + return await this.lock(async () => { + const { modeApiConfigs } = await this.load() + return { ...modeApiConfigs } + }) + } catch (error) { + throw new Error(`Failed to get mode configs: ${error}`) + } + } + public async export() { try { return await this.lock(async () => { diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index a1fc9ac7de..c77464f9f2 100644 --- a/src/core/config/__tests__/ProviderSettingsManager.spec.ts +++ b/src/core/config/__tests__/ProviderSettingsManager.spec.ts @@ -1352,6 +1352,41 @@ describe("ProviderSettingsManager", () => { }) }) + describe("GetModeConfigIds", () => { + const profilesWithModes = (modeApiConfigs?: Record) => + JSON.stringify({ + currentApiConfigName: "default", + apiConfigs: { default: { id: "default" } }, + modeApiConfigs, + migrations: { rateLimitSecondsMigrated: true }, + }) + + it("returns a copy of the stored mode map", async () => { + mockSecrets.get.mockResolvedValue(profilesWithModes({ code: "code-id", architect: "architect-id" })) + + const ids = await providerSettingsManager.getModeConfigIds() + ids.code = "changed" + + expect(ids).toEqual({ code: "changed", architect: "architect-id" }) + await expect(providerSettingsManager.getModeConfigIds()).resolves.toEqual({ + code: "code-id", + architect: "architect-id", + }) + }) + + it("returns an empty map when none is stored", async () => { + mockSecrets.get.mockResolvedValue(profilesWithModes(undefined)) + + await expect(providerSettingsManager.getModeConfigIds()).resolves.toEqual({}) + }) + + it("should throw error if secrets storage fails", async () => { + mockSecrets.get.mockRejectedValue(new Error("Storage failed")) + + await expect(providerSettingsManager.getModeConfigIds()).rejects.toThrow("Failed to get mode configs") + }) + }) + describe("HasConfig", () => { it("should return true for existing config", async () => { const existingConfig: ProviderProfiles = { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 60806b9acd..3853603568 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -364,6 +364,13 @@ export class ClineProvider */ private readonly localModeConfigIds = new Map() + /** + * The shared mode map as it was when this window opened. Modes this window has not changed + * resolve against it, so another window's later selection cannot reach this window's tasks + * (including delegated subtasks). + */ + private initialModeConfigIds: Promise> = Promise.resolve({}) + constructor( readonly context: vscode.ExtensionContext, private readonly outputChannel: vscode.OutputChannel, @@ -400,6 +407,7 @@ export class ClineProvider this._workspaceTracker = new WorkspaceTracker(this) this.providerSettingsManager = new ProviderSettingsManager(this.context) + this.captureInitialModeConfigIds() this.customModesManager = new CustomModesManager(this.context, async () => { await this.postStateToWebviewWithoutClineMessages() @@ -1836,8 +1844,20 @@ export class ClineProvider // Provider Profile Management + private captureInitialModeConfigIds(): void { + this.initialModeConfigIds = this.providerSettingsManager.getModeConfigIds().catch((error) => { + this.log(`Failed to snapshot mode profile choices: ${error instanceof Error ? error.message : error}`) + return {} + }) + } + private async getModeConfigId(mode: Mode): Promise { - return this.localModeConfigIds.get(mode) ?? (await this.providerSettingsManager.getModeConfigId(mode)) + return ( + this.localModeConfigIds.get(mode) ?? + (await this.initialModeConfigIds)[mode] ?? + // Modes created after this window opened have no snapshot entry. + (await this.providerSettingsManager.getModeConfigId(mode)) + ) } private async setModeConfig(mode: Mode, id: string): Promise { @@ -3396,6 +3416,9 @@ export class ClineProvider await this.contextProxy.resetAllState() await this.providerSettingsManager.resetAllConfigs() + // Cached choices point at profiles that no longer exist; re-read the reset mapping. + this.localModeConfigIds.clear() + this.captureInitialModeConfigIds() await this.customModesManager.resetCustomModes() await this.removeClineFromStack() await this.postStateToWebview() diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 3121fe1046..8a5fdd26a4 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -240,6 +240,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) + // These tests stub providerSettingsManager and expect it to be consulted for mode profiles. + provider["initialModeConfigIds"] = Promise.resolve({}) // Mock providerSettingsManager ;(provider as any).providerSettingsManager = { diff --git a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts index 6472b77749..512eaf6d2a 100644 --- a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts @@ -332,6 +332,8 @@ describe("ClineProvider - Lock API Config Across Modes", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) + // These tests stub providerSettingsManager and expect it to be consulted for mode profiles. + provider["initialModeConfigIds"] = Promise.resolve({}) // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 4da68d7904..4790b243a2 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -552,6 +552,9 @@ describe("ClineProvider", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) + // Most tests stub providerSettingsManager and expect it to be consulted for mode profiles, + // so start without the snapshot the real manager would take of the seeded mode map. + provider["initialModeConfigIds"] = Promise.resolve({}) defaultTaskOptions = { provider, @@ -2462,6 +2465,53 @@ describe("ClineProvider", () => { await expect(provider["getModeConfigId"]("architect")).resolves.toBe("other-window-id") }) + describe("mode profile snapshot", () => { + it("keeps the choice this window opened with when another window changes the shared value", async () => { + provider["initialModeConfigIds"] = Promise.resolve({ architect: "opened-with-id" }) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("other-window-id") + + await expect(provider["getModeConfigId"]("architect")).resolves.toBe("opened-with-id") + // A mode created after this window opened has no snapshot entry, so the shared value applies. + await expect(provider["getModeConfigId"]("new-custom-mode")).resolves.toBe("other-window-id") + }) + + it("prefers an explicit local choice over the snapshot", async () => { + provider["initialModeConfigIds"] = Promise.resolve({ architect: "opened-with-id" }) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + + await provider["setModeConfig"]("architect", "chosen-id") + + await expect(provider["getModeConfigId"]("architect")).resolves.toBe("chosen-id") + }) + + it("falls back to the shared value when the snapshot cannot be read", async () => { + vi.spyOn(provider.providerSettingsManager, "getModeConfigIds").mockRejectedValue(new Error("unreadable")) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("shared-id") + + provider["captureInitialModeConfigIds"]() + + await expect(provider["getModeConfigId"]("architect")).resolves.toBe("shared-id") + }) + + it("drops local choices and re-reads the mapping when state is reset", async () => { + const { t } = await import("../../../i18n") + vi.mocked(vscode.window.showInformationMessage).mockResolvedValueOnce(t("common:answers.yes") as never) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "resetAllConfigs").mockResolvedValue(undefined) + vi.spyOn(provider.contextProxy, "resetAllState").mockResolvedValue(undefined) + // The shared custom-modes mock does not implement reset. + Object.assign(provider.customModesManager, { resetCustomModes: vi.fn().mockResolvedValue(undefined) }) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "getModeConfigIds").mockResolvedValue({ code: "reset-id" }) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue(undefined) + await provider["setModeConfig"]("code", "removed-profile-id") + + await provider.resetState() + + await expect(provider["getModeConfigId"]("code")).resolves.toBe("reset-id") + }) + }) + it("saves current config when switching to mode without config", async () => { await provider.resolveWebviewView(mockWebviewView) const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] From ed2416c18414e0013cc3d88a4f5b1a97377a2527 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 4 Nov 2026 15:41:23 +0100 Subject: [PATCH 4/9] fix(profiles): keep unseen modes window-local, drop stale choices, roll back failed writes --- src/core/webview/ClineProvider.ts | 44 +++++++++--- .../ClineProvider.apiHandlerRebuild.spec.ts | 4 +- .../ClineProvider.lockApiConfig.spec.ts | 4 +- .../webview/__tests__/ClineProvider.spec.ts | 69 ++++++++++++++----- 4 files changed, 91 insertions(+), 30 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8b8a7087af..42b9755665 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -430,9 +430,9 @@ export class ClineProvider /** * The shared mode map as it was when this window opened. Modes this window has not changed * resolve against it, so another window's later selection cannot reach this window's tasks - * (including delegated subtasks). + * (including delegated subtasks). `undefined` when the map could not be read. */ - private initialModeConfigIds: Promise> = Promise.resolve({}) + private initialModeConfigIds: Promise | undefined> = Promise.resolve({}) constructor( readonly context: vscode.ExtensionContext, @@ -2169,22 +2169,46 @@ export class ClineProvider private captureInitialModeConfigIds(): void { this.initialModeConfigIds = this.providerSettingsManager.getModeConfigIds().catch((error) => { this.log(`Failed to snapshot mode profile choices: ${error instanceof Error ? error.message : error}`) - return {} + return undefined }) } + /** + * Resolve the profile id for `mode` from this window's own view: its explicit choices, then the + * shared map as it was when the window opened. Modes absent from both resolve to `undefined`, so + * callers keep this window's current profile instead of one another window saved later. A cached + * id whose profile no longer exists (e.g. after a reset) is dropped in favour of the shared map, + * which is also used when the opening snapshot could not be read. + */ private async getModeConfigId(mode: Mode): Promise { - return ( - this.localModeConfigIds.get(mode) ?? - (await this.initialModeConfigIds)[mode] ?? - // Modes created after this window opened have no snapshot entry. - (await this.providerSettingsManager.getModeConfigId(mode)) - ) + const localId = this.localModeConfigIds.get(mode) + const initialIds = localId === undefined ? await this.initialModeConfigIds : undefined + if (localId === undefined && initialIds === undefined) { + return this.providerSettingsManager.getModeConfigId(mode) + } + const cachedId = localId ?? initialIds?.[mode] + if (cachedId === undefined) return undefined + + const profiles = await this.providerSettingsManager.listConfig() + if (profiles.some(({ id }) => id === cachedId)) return cachedId + + if (localId !== undefined) this.localModeConfigIds.delete(mode) + return this.providerSettingsManager.getModeConfigId(mode) } private async setModeConfig(mode: Mode, id: string): Promise { + const previousId = this.localModeConfigIds.get(mode) this.localModeConfigIds.set(mode, id) - await this.providerSettingsManager.setModeConfig(mode, id) + try { + await this.providerSettingsManager.setModeConfig(mode, id) + } catch (error) { + // Keep this window consistent with the shared map the failed write left unchanged. + if (this.localModeConfigIds.get(mode) === id) { + if (previousId === undefined) this.localModeConfigIds.delete(mode) + else this.localModeConfigIds.set(mode, previousId) + } + throw error + } } /** diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 3d8beb72fa..8fb0504c33 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -239,8 +239,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) - // These tests stub providerSettingsManager and expect it to be consulted for mode profiles. - provider["initialModeConfigIds"] = Promise.resolve({}) + // These tests stub providerSettingsManager; without an opening snapshot, mode lookups read it. + provider["initialModeConfigIds"] = Promise.resolve(undefined) // Mock providerSettingsManager ;(provider as any).providerSettingsManager = { diff --git a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts index 62514466cb..6e22e35902 100644 --- a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts @@ -331,8 +331,8 @@ describe("ClineProvider - Lock API Config Across Modes", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) - // These tests stub providerSettingsManager and expect it to be consulted for mode profiles. - provider["initialModeConfigIds"] = Promise.resolve({}) + // These tests stub providerSettingsManager; without an opening snapshot, mode lookups read it. + provider["initialModeConfigIds"] = Promise.resolve(undefined) // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 8bf4ea5c9d..b702a0de2e 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -560,7 +560,7 @@ describe("ClineProvider", () => { ) // Most tests stub providerSettingsManager and expect it to be consulted for mode profiles, // so start without the snapshot the real manager would take of the seeded mode map. - provider["initialModeConfigIds"] = Promise.resolve({}) + provider["initialModeConfigIds"] = Promise.resolve(undefined) defaultTaskOptions = { provider, @@ -4481,33 +4481,41 @@ describe("ClineProvider", () => { expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") }) - it("prefers this window's mode profile over one another window saved in shared storage", async () => { - await provider.resolveWebviewView(mockWebviewView) + describe("mode profile choices", () => { + const existingProfiles = (...ids: string[]) => + vi + .spyOn(provider.providerSettingsManager, "listConfig") + .mockResolvedValue(ids.map((id) => ({ name: `${id}-name`, id }))) - const manager = provider.providerSettingsManager - vi.spyOn(manager, "getModeConfigId").mockResolvedValue("other-window-id") - vi.spyOn(manager, "setModeConfig").mockResolvedValue(undefined) + it("prefers this window's mode profile over one another window saved in shared storage", async () => { + const manager = provider.providerSettingsManager + existingProfiles("this-window-id", "other-window-id") + vi.spyOn(manager, "getModeConfigId").mockResolvedValue("other-window-id") + vi.spyOn(manager, "setModeConfig").mockResolvedValue(undefined) - await provider["setModeConfig"]("code", "this-window-id") + await provider["setModeConfig"]("code", "this-window-id") - expect(manager.setModeConfig).toHaveBeenCalledWith("code", "this-window-id") - await expect(provider["getModeConfigId"]("code")).resolves.toBe("this-window-id") - // A mode this window never chose still falls back to the shared value. - await expect(provider["getModeConfigId"]("architect")).resolves.toBe("other-window-id") - }) + expect(manager.setModeConfig).toHaveBeenCalledWith("code", "this-window-id") + await expect(provider["getModeConfigId"]("code")).resolves.toBe("this-window-id") + }) - describe("mode profile snapshot", () => { it("keeps the choice this window opened with when another window changes the shared value", async () => { provider["initialModeConfigIds"] = Promise.resolve({ architect: "opened-with-id" }) - vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("other-window-id") + existingProfiles("opened-with-id", "other-window-id") + const sharedLookup = vi + .spyOn(provider.providerSettingsManager, "getModeConfigId") + .mockResolvedValue("other-window-id") await expect(provider["getModeConfigId"]("architect")).resolves.toBe("opened-with-id") - // A mode created after this window opened has no snapshot entry, so the shared value applies. - await expect(provider["getModeConfigId"]("new-custom-mode")).resolves.toBe("other-window-id") + // A mode absent when this window opened keeps this window's current profile instead of + // one another window saved for it later. + await expect(provider["getModeConfigId"]("new-custom-mode")).resolves.toBeUndefined() + expect(sharedLookup).not.toHaveBeenCalled() }) it("prefers an explicit local choice over the snapshot", async () => { provider["initialModeConfigIds"] = Promise.resolve({ architect: "opened-with-id" }) + existingProfiles("opened-with-id", "chosen-id") vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) await provider["setModeConfig"]("architect", "chosen-id") @@ -4524,6 +4532,34 @@ describe("ClineProvider", () => { await expect(provider["getModeConfigId"]("architect")).resolves.toBe("shared-id") }) + it("drops a local choice whose profile was removed and falls back to the shared mapping", async () => { + provider["initialModeConfigIds"] = Promise.resolve({}) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("shared-id") + await provider["setModeConfig"]("code", "removed-id") + // Another window reset or deleted the profile this window chose. + existingProfiles("shared-id") + + await expect(provider["getModeConfigId"]("code")).resolves.toBe("shared-id") + expect(provider["localModeConfigIds"].has("code")).toBe(false) + }) + + it("restores the previous local choice when the shared write fails", async () => { + provider["initialModeConfigIds"] = Promise.resolve({}) + existingProfiles("first-id", "second-id") + const sharedWrite = vi.spyOn(provider.providerSettingsManager, "setModeConfig") + sharedWrite.mockResolvedValueOnce(undefined) + await provider["setModeConfig"]("code", "first-id") + + sharedWrite.mockRejectedValueOnce(new Error("write failed")) + await expect(provider["setModeConfig"]("code", "second-id")).rejects.toThrow("write failed") + await expect(provider["getModeConfigId"]("code")).resolves.toBe("first-id") + + sharedWrite.mockRejectedValueOnce(new Error("write failed")) + await expect(provider["setModeConfig"]("architect", "second-id")).rejects.toThrow("write failed") + await expect(provider["getModeConfigId"]("architect")).resolves.toBeUndefined() + }) + it("drops local choices and re-reads the mapping when state is reset", async () => { const { t } = await import("../../../i18n") vi.mocked(vscode.window.showInformationMessage).mockResolvedValueOnce(t("common:answers.yes") as never) @@ -4535,6 +4571,7 @@ describe("ClineProvider", () => { vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) vi.spyOn(provider.providerSettingsManager, "getModeConfigIds").mockResolvedValue({ code: "reset-id" }) vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue(undefined) + existingProfiles("removed-profile-id", "reset-id") await provider["setModeConfig"]("code", "removed-profile-id") await provider.resetState() From 4ba97ac716867e0fe64f3f997b4f67e302e61423 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Sat, 7 Nov 2026 15:37:03 +0000 Subject: [PATCH 5/9] fix: preserve window-local mode defaults after imports and API updates --- .../config/__tests__/importExport.spec.ts | 28 +++++++++++++++++++ src/core/config/importExport.ts | 24 +++++++++++++--- src/core/webview/ClineProvider.ts | 9 +++++- .../webview/__tests__/ClineProvider.spec.ts | 13 +++++++++ src/extension.ts | 1 + .../__tests__/api-configuration.spec.ts | 26 +++++++++++++---- src/extension/api.ts | 2 +- src/utils/autoImportSettings.ts | 3 +- 8 files changed, 94 insertions(+), 12 deletions(-) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 6c66655325..de654db60c 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -194,6 +194,29 @@ describe("importExport", () => { mockContextProxy.setProviderSettings.mockResolvedValue(undefined) }) + it.each([false, true])( + "updates the importing window only after successful persistence (failure: %s)", + async (fails) => { + const modeApiConfigs = { "new-custom-mode": "test-id" } + const file = JSON.parse(validFileContent) + file.providerProfiles.modeApiConfigs = modeApiConfigs + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(file)) + const onModeConfigIdsImported = vi.fn() + if (fails) mockProviderSettingsManager.import.mockRejectedValueOnce(new Error("write failed")) + + const result = await importSettingsFromPath("/mock/path/settings.json", { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + onModeConfigIdsImported, + }) + + expect(result.success).toBe(!fails) + if (fails) expect(onModeConfigIdsImported).not.toHaveBeenCalled() + else expect(onModeConfigIdsImported).toHaveBeenCalledExactlyOnceWith(modeApiConfigs) + }, + ) + it("waits for each provider-state write before reporting success", async () => { const currentProfileWrite = createDeferred() const providerSettingsWrite = createDeferred() @@ -638,6 +661,7 @@ describe("importExport", () => { // Create a mock provider for the test const mockProvider = { settingsImportedAt: 0, + adoptModeConfigIds: vi.fn(), postStateToWebview: vi.fn().mockResolvedValue(undefined), postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined), } @@ -921,6 +945,7 @@ describe("importExport", () => { const seenImportedAt: Array = [] const mockProvider = { settingsImportedAt: undefined as number | undefined, + adoptModeConfigIds: vi.fn(), postStateToWebview: vi.fn().mockImplementation(async () => { seenImportedAt.push(mockProvider.settingsImportedAt) }), @@ -997,6 +1022,7 @@ describe("importExport", () => { const seenImportedAt: Array = [] const mockProvider = { settingsImportedAt: undefined as number | undefined, + adoptModeConfigIds: vi.fn(), postStateToWebview: vi.fn().mockImplementation(async () => { seenImportedAt.push(mockProvider.settingsImportedAt) }), @@ -1268,6 +1294,7 @@ describe("importExport", () => { const mockProvider = { settingsImportedAt: 0, + adoptModeConfigIds: vi.fn(), postStateToWebview: vi.fn().mockResolvedValue(undefined), } @@ -1537,6 +1564,7 @@ describe("importExport", () => { const mockProvider = { settingsImportedAt: 0, + adoptModeConfigIds: vi.fn(), postStateToWebview: vi.fn().mockResolvedValue(undefined), } diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 9882039ad3..df9281804d 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -27,6 +27,7 @@ export type ImportOptions = { providerSettingsManager: ProviderSettingsManager contextProxy: ContextProxy customModesManager: CustomModesManager + onModeConfigIdsImported?: (ids: Record) => void } type ExportOptions = { @@ -37,6 +38,7 @@ type ImportWithProviderOptions = ImportOptions & { provider: { settingsImportedAt?: number postStateToWebview: () => Promise + adoptModeConfigIds: (ids: Record) => void } } @@ -135,7 +137,7 @@ function sanitizeGlobalSettings(rawGlobalSettings: unknown): { */ export async function importSettingsFromPath( filePath: string, - { providerSettingsManager, contextProxy, customModesManager }: ImportOptions, + { providerSettingsManager, contextProxy, customModesManager, onModeConfigIdsImported }: ImportOptions, ) { // Use a lenient schema that accepts any apiConfigs, then validate each individually const lenientProviderProfilesSchema = providerProfilesSchema.extend({ @@ -228,6 +230,7 @@ export async function importSettingsFromPath( // They will be imported automatically with the config - no special handling needed await providerSettingsManager.import(providerProfiles) + onModeConfigIdsImported?.(rawProviderProfiles.modeApiConfigs ?? {}) await contextProxy.setValues(sanitizedGlobalSettings) // Set the current provider. @@ -269,7 +272,12 @@ export async function importSettingsFromPath( * @param options - Import options containing managers and proxy * @returns Promise resolving to import result */ -export const importSettings = async ({ providerSettingsManager, contextProxy, customModesManager }: ImportOptions) => { +export const importSettings = async ({ + providerSettingsManager, + contextProxy, + customModesManager, + onModeConfigIdsImported, +}: ImportOptions) => { // Use the last export path as a sensible default, falling back to Downloads const defaultUri = resolveDefaultSaveUri(contextProxy, "lastSettingsExportPath", "zoo-code-settings.json", { useWorkspace: false, @@ -290,6 +298,7 @@ export const importSettings = async ({ providerSettingsManager, contextProxy, cu providerSettingsManager, contextProxy, customModesManager, + onModeConfigIdsImported, }) } @@ -302,13 +311,14 @@ export const importSettings = async ({ providerSettingsManager, contextProxy, cu * @returns Promise resolving to import result */ export const importSettingsFromFile = async ( - { providerSettingsManager, contextProxy, customModesManager }: ImportOptions, + { providerSettingsManager, contextProxy, customModesManager, onModeConfigIdsImported }: ImportOptions, fileUri: vscode.Uri, ) => { return importSettingsFromPath(fileUri.fsPath, { providerSettingsManager, contextProxy, customModesManager, + onModeConfigIdsImported, }) } @@ -374,6 +384,7 @@ export const importSettingsWithFeedback = async ( providerSettingsManager, contextProxy, customModesManager, + onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids), }) } catch (error) { result = { @@ -382,7 +393,12 @@ export const importSettingsWithFeedback = async ( } } } else { - result = await importSettings({ providerSettingsManager, contextProxy, customModesManager }) + result = await importSettings({ + providerSettingsManager, + contextProxy, + customModesManager, + onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids), + }) } if (result.success) { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 42b9755665..cd6a8789ad 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2196,7 +2196,14 @@ export class ClineProvider return this.providerSettingsManager.getModeConfigId(mode) } - private async setModeConfig(mode: Mode, id: string): Promise { + /** Adopt only mappings explicitly written by an import in this window. */ + public adoptModeConfigIds(ids: Record): void { + for (const [mode, id] of Object.entries(ids)) { + this.localModeConfigIds.set(mode, id) + } + } + + public async setModeConfig(mode: Mode, id: string): Promise { const previousId = this.localModeConfigIds.get(mode) this.localModeConfigIds.set(mode, id) try { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index b702a0de2e..9aca6f1eb9 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -4523,6 +4523,19 @@ describe("ClineProvider", () => { await expect(provider["getModeConfigId"]("architect")).resolves.toBe("chosen-id") }) + it.each(["architect", "new-custom-mode"])( + "adopts imported defaults for %s without adopting other windows' writes", + async (mode) => { + provider["initialModeConfigIds"] = Promise.resolve({ architect: "opened-with-id" }) + existingProfiles("opened-with-id", "imported-id", "other-window-id") + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("other-window-id") + provider.adoptModeConfigIds({ [mode]: "imported-id" }) + + await expect(provider["getModeConfigId"](mode)).resolves.toBe("imported-id") + await expect(provider["getModeConfigId"]("unrelated-mode")).resolves.toBeUndefined() + }, + ) + it("falls back to the shared value when the snapshot cannot be read", async () => { vi.spyOn(provider.providerSettingsManager, "getModeConfigIds").mockRejectedValue(new Error("unreadable")) vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("shared-id") diff --git a/src/extension.ts b/src/extension.ts index 6ce42aacd5..07f6089a03 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -283,6 +283,7 @@ export async function activate(context: vscode.ExtensionContext) { providerSettingsManager: provider.providerSettingsManager, contextProxy: provider.contextProxy, customModesManager: provider.customModesManager, + onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids), }) } catch (error) { outputChannel.appendLine( diff --git a/src/extension/__tests__/api-configuration.spec.ts b/src/extension/__tests__/api-configuration.spec.ts index f4ed8af469..3b19fc0f2c 100644 --- a/src/extension/__tests__/api-configuration.spec.ts +++ b/src/extension/__tests__/api-configuration.spec.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest" import type * as vscode from "vscode" import { API } from "../api" -import type { ClineProvider } from "../../core/webview/ClineProvider" +import { ClineProvider } from "../../core/webview/ClineProvider" import { makeClineProviderFactory } from "../../test-utils/provider" vi.mock("@roo-code/ipc", () => ({ @@ -19,15 +19,27 @@ describe("API - configuration", () => { context: {}, on: vi.fn(), contextProxy: { setValues }, - providerSettingsManager: { saveConfig, setModeConfig }, + providerSettingsManager: { + saveConfig, + setModeConfig, + listConfig: vi.fn().mockResolvedValue([ + { name: "code", id: "code-config" }, + { name: "custom", id: "architect-config" }, + ]), + }, + localModeConfigIds: new Map(), + initialModeConfigIds: Promise.resolve({}), + setModeConfig: ClineProvider.prototype.setModeConfig, + getModeConfigId: ClineProvider.prototype["getModeConfigId"], postStateToWebview, + // Provides the configuration methods and event registration used by API. } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.setConfiguration({ currentApiConfigName: "default", - modeApiConfigs: { code: "code-config", architect: "architect-config" }, + modeApiConfigs: { code: "code-config", "new-custom-mode": "architect-config" }, }) expect(saveConfig).toHaveBeenCalledWith("default", expect.objectContaining({ currentApiConfigName: "default" })) @@ -39,7 +51,9 @@ describe("API - configuration", () => { ) expect(setModeConfig).toHaveBeenCalledTimes(2) expect(setModeConfig).toHaveBeenCalledWith("code", "code-config") - expect(setModeConfig).toHaveBeenCalledWith("architect", "architect-config") + expect(setModeConfig).toHaveBeenCalledWith("new-custom-mode", "architect-config") + await expect(provider["getModeConfigId"]("code")).resolves.toBe("code-config") + await expect(provider["getModeConfigId"]("new-custom-mode")).resolves.toBe("architect-config") expect(postStateToWebview).toHaveBeenCalledOnce() }) @@ -52,8 +66,10 @@ describe("API - configuration", () => { context: {}, on: vi.fn(), contextProxy: { setValues }, - providerSettingsManager: { saveConfig, setModeConfig }, + providerSettingsManager: { saveConfig }, + setModeConfig, postStateToWebview, + // Provides the configuration methods and event registration used by API. } as unknown as ClineProvider const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel const api = new API(outputChannel, provider, makeClineProviderFactory()) diff --git a/src/extension/api.ts b/src/extension/api.ts index da59c48b58..60110919a7 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -634,7 +634,7 @@ export class API extends EventEmitter implements RooCodeAPI { if (values.modeApiConfigs) { await Promise.all( Object.entries(values.modeApiConfigs).map(([mode, configId]) => - this.sidebarProvider.providerSettingsManager.setModeConfig(mode as Mode, configId), + this.sidebarProvider.setModeConfig(mode as Mode, configId), ), ) } diff --git a/src/utils/autoImportSettings.ts b/src/utils/autoImportSettings.ts index 3c8773fb6b..511bec72c3 100644 --- a/src/utils/autoImportSettings.ts +++ b/src/utils/autoImportSettings.ts @@ -15,7 +15,7 @@ import { importSettingsFromPath, ImportOptions } from "../core/config/importExpo */ export async function autoImportSettings( outputChannel: vscode.OutputChannel, - { providerSettingsManager, contextProxy, customModesManager }: ImportOptions, + { providerSettingsManager, contextProxy, customModesManager, onModeConfigIdsImported }: ImportOptions, ): Promise { try { // Get the auto-import settings path from VSCode settings @@ -41,6 +41,7 @@ export async function autoImportSettings( providerSettingsManager, contextProxy, customModesManager, + onModeConfigIdsImported, }) if (result.success) { From 19d3b3c2b7e336ac35271b8db0f4db72b0fdab0a Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Mon, 9 Nov 2026 05:26:33 +0000 Subject: [PATCH 6/9] fix: preserve newer local profile choices during stale ID cleanup --- src/core/webview/ClineProvider.ts | 3 ++ .../webview/__tests__/ClineProvider.spec.ts | 36 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index cd6a8789ad..3762b67ce2 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2183,6 +2183,7 @@ export class ClineProvider private async getModeConfigId(mode: Mode): Promise { const localId = this.localModeConfigIds.get(mode) const initialIds = localId === undefined ? await this.initialModeConfigIds : undefined + if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode) if (localId === undefined && initialIds === undefined) { return this.providerSettingsManager.getModeConfigId(mode) } @@ -2190,6 +2191,8 @@ export class ClineProvider if (cachedId === undefined) return undefined const profiles = await this.providerSettingsManager.listConfig() + // A profile selection may have changed while either asynchronous read was pending. + if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode) if (profiles.some(({ id }) => id === cachedId)) return cachedId if (localId !== undefined) this.localModeConfigIds.delete(mode) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 9aca6f1eb9..0baa61a23f 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -4557,6 +4557,42 @@ describe("ClineProvider", () => { expect(provider["localModeConfigIds"].has("code")).toBe(false) }) + it.each(["removed-id", "existing-id"])( + "preserves a newer local choice while validating %s", + async (previousId) => { + provider["initialModeConfigIds"] = Promise.resolve({ code: "opened-with-id" }) + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + const sharedLookup = vi + .spyOn(provider.providerSettingsManager, "getModeConfigId") + .mockResolvedValue("other-window-id") + await provider.setModeConfig("code", previousId) + let releaseLookup = () => {} + const pendingLookup = new Promise((resolve) => { + releaseLookup = resolve + }) + const profiles = [ + { name: "existing", id: "existing-id" }, + { name: "new", id: "new-id" }, + ] + const listConfig = vi.spyOn(provider.providerSettingsManager, "listConfig") + listConfig + .mockImplementationOnce(async () => { + await pendingLookup + return profiles + }) + .mockResolvedValue(profiles) + + const lookup = provider["getModeConfigId"]("code") + expect(listConfig).toHaveBeenCalledOnce() + await provider.setModeConfig("code", "new-id") + releaseLookup() + + await expect(lookup).resolves.toBe("new-id") + await expect(provider["getModeConfigId"]("code")).resolves.toBe("new-id") + expect(sharedLookup).not.toHaveBeenCalled() + }, + ) + it("restores the previous local choice when the shared write fails", async () => { provider["initialModeConfigIds"] = Promise.resolve({}) existingProfiles("first-id", "second-id") From 6edefcec7d0d37ed576bb0fb19b1304b4edf3768 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Mon, 9 Nov 2026 23:14:18 +0000 Subject: [PATCH 7/9] test: cover mode profile selection during pending snapshot --- .../webview/__tests__/ClineProvider.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 0baa61a23f..a828e64497 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -4557,6 +4557,25 @@ describe("ClineProvider", () => { expect(provider["localModeConfigIds"].has("code")).toBe(false) }) + it("preserves a local choice made while the initial snapshot is pending", async () => { + let resolveSnapshot = (_ids: Record) => {} + provider["initialModeConfigIds"] = new Promise>((resolve) => { + resolveSnapshot = resolve + }) + existingProfiles("new-id") + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + const sharedLookup = vi + .spyOn(provider.providerSettingsManager, "getModeConfigId") + .mockResolvedValue("other-window-id") + + const lookup = provider["getModeConfigId"]("code") + await provider.setModeConfig("code", "new-id") + resolveSnapshot({}) + + await expect(lookup).resolves.toBe("new-id") + expect(sharedLookup).not.toHaveBeenCalled() + }) + it.each(["removed-id", "existing-id"])( "preserves a newer local choice while validating %s", async (previousId) => { From 68160ec694a567499f3ca89ee3218c0337c6338c Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 10 Nov 2026 00:22:19 +0000 Subject: [PATCH 8/9] fix: isolate mode profiles on snapshot failure and serialize writes --- src/core/webview/ClineProvider.ts | 23 +++++++-- .../ClineProvider.apiHandlerRebuild.spec.ts | 5 +- .../ClineProvider.lockApiConfig.spec.ts | 7 +-- .../webview/__tests__/ClineProvider.spec.ts | 47 ++++++++++++++++--- .../__tests__/api-configuration.spec.ts | 2 + 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3762b67ce2..3d46f26e02 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -434,6 +434,9 @@ export class ClineProvider */ private initialModeConfigIds: Promise | undefined> = Promise.resolve({}) + /** Serialize each mode’s local choice and persisted write, including failure rollback. */ + private readonly modeConfigWrites = new Map>() + constructor( readonly context: vscode.ExtensionContext, private readonly outputChannel: vscode.OutputChannel, @@ -2178,15 +2181,12 @@ export class ClineProvider * shared map as it was when the window opened. Modes absent from both resolve to `undefined`, so * callers keep this window's current profile instead of one another window saved later. A cached * id whose profile no longer exists (e.g. after a reset) is dropped in favour of the shared map, - * which is also used when the opening snapshot could not be read. + * while an unreadable opening snapshot keeps this window’s current profile. */ private async getModeConfigId(mode: Mode): Promise { const localId = this.localModeConfigIds.get(mode) const initialIds = localId === undefined ? await this.initialModeConfigIds : undefined if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode) - if (localId === undefined && initialIds === undefined) { - return this.providerSettingsManager.getModeConfigId(mode) - } const cachedId = localId ?? initialIds?.[mode] if (cachedId === undefined) return undefined @@ -2207,6 +2207,21 @@ export class ClineProvider } public async setModeConfig(mode: Mode, id: string): Promise { + const pending = this.modeConfigWrites.get(mode) + const write = (async () => { + // A failed predecessor must roll back before this operation captures its previous choice. + if (pending) await pending.catch(() => {}) + await this.writeModeConfig(mode, id) + })() + this.modeConfigWrites.set(mode, write) + try { + await write + } finally { + if (this.modeConfigWrites.get(mode) === write) this.modeConfigWrites.delete(mode) + } + } + + private async writeModeConfig(mode: Mode, id: string): Promise { const previousId = this.localModeConfigIds.get(mode) this.localModeConfigIds.set(mode, id) try { diff --git a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts index 8fb0504c33..ca504be775 100644 --- a/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts @@ -239,8 +239,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) - // These tests stub providerSettingsManager; without an opening snapshot, mode lookups read it. - provider["initialModeConfigIds"] = Promise.resolve(undefined) + // Tests provide explicit opening snapshots when they need saved mode profiles. + provider["initialModeConfigIds"] = Promise.resolve({}) // Mock providerSettingsManager ;(provider as any).providerSettingsManager = { @@ -977,6 +977,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => { }, }) await provider.addClineToStack(mockTask) + provider["initialModeConfigIds"] = Promise.resolve({ ask: "ask-id" }) provider["providerSettingsManager"].getModeConfigId = vi.fn().mockResolvedValue("ask-id") provider["providerSettingsManager"].listConfig = vi .fn() diff --git a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts index 6e22e35902..7c85e90e76 100644 --- a/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts @@ -331,8 +331,8 @@ describe("ClineProvider - Lock API Config Across Modes", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) - // These tests stub providerSettingsManager; without an opening snapshot, mode lookups read it. - provider["initialModeConfigIds"] = Promise.resolve(undefined) + // Tests provide explicit opening snapshots when they need saved mode profiles. + provider["initialModeConfigIds"] = Promise.resolve({}) // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ @@ -392,9 +392,10 @@ describe("ClineProvider - Lock API Config Across Modes", () => { apiProvider: providerIdentifiers.anthropic, }) + provider["initialModeConfigIds"] = Promise.resolve({ architect: "architect-profile-id" }) await provider.handleModeSwitch("architect") - expect(getModeConfigIdSpy).toHaveBeenCalledWith("architect") + expect(getModeConfigIdSpy).not.toHaveBeenCalled() expect(activateProfileSpy).toHaveBeenCalledWith({ name: "architect-profile" }) }) }) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index a828e64497..a11048d7fb 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -558,9 +558,8 @@ describe("ClineProvider", () => { new ContextProxy(mockContext), new WebviewFocusTracker(), ) - // Most tests stub providerSettingsManager and expect it to be consulted for mode profiles, - // so start without the snapshot the real manager would take of the seeded mode map. - provider["initialModeConfigIds"] = Promise.resolve(undefined) + // Tests provide explicit opening snapshots when they need saved mode profiles. + provider["initialModeConfigIds"] = Promise.resolve({}) defaultTaskOptions = { provider, @@ -4464,6 +4463,7 @@ describe("ClineProvider", () => { apiProvider: providerIdentifiers.anthropic, } + provider["initialModeConfigIds"] = Promise.resolve({ architect: "test-id" }) ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("test-id"), listConfig: vi.fn().mockResolvedValue([profile]), @@ -4476,7 +4476,7 @@ describe("ClineProvider", () => { await messageHandler({ type: "mode", text: "architect" }) // Should load the saved config for architect mode - expect(provider.providerSettingsManager.getModeConfigId).toHaveBeenCalledWith("architect") + expect(provider.providerSettingsManager.getModeConfigId).not.toHaveBeenCalled() expect(provider.providerSettingsManager.activateProfile).toHaveBeenCalledWith({ name: "test-config" }) expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "test-config") }) @@ -4536,13 +4536,18 @@ describe("ClineProvider", () => { }, ) - it("falls back to the shared value when the snapshot cannot be read", async () => { + it("keeps the current profile when the snapshot cannot be read", async () => { vi.spyOn(provider.providerSettingsManager, "getModeConfigIds").mockRejectedValue(new Error("unreadable")) vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("shared-id") provider["captureInitialModeConfigIds"]() - await expect(provider["getModeConfigId"]("architect")).resolves.toBe("shared-id") + await expect(provider["getModeConfigId"]("architect")).resolves.toBeUndefined() + expect(provider.providerSettingsManager.getModeConfigId).not.toHaveBeenCalled() + existingProfiles("chosen-id") + vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue(undefined) + await provider.setModeConfig("architect", "chosen-id") + await expect(provider["getModeConfigId"]("architect")).resolves.toBe("chosen-id") }) it("drops a local choice whose profile was removed and falls back to the shared mapping", async () => { @@ -4628,6 +4633,31 @@ describe("ClineProvider", () => { await expect(provider["getModeConfigId"]("architect")).resolves.toBeUndefined() }) + it.each(["first-id", "second-id"])( + "preserves a successful overlapping write of %s after the preceding write fails", + async (nextId) => { + existingProfiles("first-id", "second-id") + const sharedWrite = vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue() + await provider.setModeConfig("code", "first-id") + let rejectWrite = (_error: Error) => {} + sharedWrite.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectWrite = reject + }), + ) + const failed = provider.setModeConfig("code", "second-id") + const failure = expect(failed).rejects.toThrow("write failed") + const succeeded = provider.setModeConfig("code", nextId) + expect(sharedWrite).toHaveBeenCalledTimes(2) + rejectWrite(new Error("write failed")) + await failure + await succeeded + expect(sharedWrite).toHaveBeenLastCalledWith("code", nextId) + await expect(provider["getModeConfigId"]("code")).resolves.toBe(nextId) + }, + ) + it("drops local choices and re-reads the mapping when state is reset", async () => { const { t } = await import("../../../i18n") vi.mocked(vscode.window.showInformationMessage).mockResolvedValueOnce(t("common:answers.yes") as never) @@ -5234,6 +5264,7 @@ describe("ClineProvider", () => { apiProvider: providerIdentifiers.anthropic, } + provider["initialModeConfigIds"] = Promise.resolve({ architect: "saved-config-id" }) ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("saved-config-id"), listConfig: vi.fn().mockResolvedValue([profile]), @@ -5249,7 +5280,7 @@ describe("ClineProvider", () => { expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "architect") // Verify saved config was loaded - expect(provider.providerSettingsManager.getModeConfigId).toHaveBeenCalledWith("architect") + expect(provider.providerSettingsManager.getModeConfigId).not.toHaveBeenCalled() expect(provider.providerSettingsManager.activateProfile).toHaveBeenCalledWith({ name: "saved-config" }) expect(mockContext.globalState.update).toHaveBeenCalledWith("currentApiConfigName", "saved-config") @@ -5384,6 +5415,7 @@ describe("ClineProvider", () => { }) // Mock provider settings manager + provider["initialModeConfigIds"] = Promise.resolve({ "custom-mode": "config-id" }) ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("config-id"), listConfig: vi @@ -5523,6 +5555,7 @@ describe("ClineProvider", () => { }) // Mock provider settings manager to throw error + provider["initialModeConfigIds"] = Promise.resolve({ code: "config-id" }) ;(provider as any).providerSettingsManager = { getModeConfigId: vi.fn().mockResolvedValue("config-id"), listConfig: vi diff --git a/src/extension/__tests__/api-configuration.spec.ts b/src/extension/__tests__/api-configuration.spec.ts index 3b19fc0f2c..31b0be733c 100644 --- a/src/extension/__tests__/api-configuration.spec.ts +++ b/src/extension/__tests__/api-configuration.spec.ts @@ -28,8 +28,10 @@ describe("API - configuration", () => { ]), }, localModeConfigIds: new Map(), + modeConfigWrites: new Map>(), initialModeConfigIds: Promise.resolve({}), setModeConfig: ClineProvider.prototype.setModeConfig, + writeModeConfig: ClineProvider.prototype["writeModeConfig"], getModeConfigId: ClineProvider.prototype["getModeConfigId"], postStateToWebview, // Provides the configuration methods and event registration used by API. From e4d28bb6db329ba5a1def308485c59ad1def4869 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 10 Nov 2026 01:41:52 +0000 Subject: [PATCH 9/9] fix: preserve mode profile ordering during imports and stop cancelled lookups --- src/core/config/ProviderSettingsManager.ts | 11 +- .../__tests__/ProviderSettingsManager.spec.ts | 30 +++ .../config/__tests__/importExport.spec.ts | 4 +- src/core/config/importExport.ts | 9 +- src/core/webview/ClineProvider.ts | 19 +- .../webview/__tests__/ClineProvider.spec.ts | 171 +++++++++++++++++- .../ClineProvider.sticky-profile.spec.ts | 6 +- .../__tests__/api-configuration.spec.ts | 4 +- 8 files changed, 231 insertions(+), 23 deletions(-) diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index a355b2ff01..058ed63483 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -543,7 +543,7 @@ export class ProviderSettingsManager { /** * Set the API config for a specific mode. */ - public async setModeConfig(mode: Mode, configId: string) { + public async setModeConfig(mode: Mode, configId: string, onPersisted?: () => void) { try { return await this.lock(async () => { const providerProfiles = await this.load() @@ -554,6 +554,7 @@ export class ProviderSettingsManager { // Assign the chosen config ID to this mode providerProfiles.modeApiConfigs[mode] = configId await this.store(providerProfiles) + onPersisted?.() }) } catch (error) { throw new Error(`Failed to set mode config: ${error}`) @@ -651,9 +652,13 @@ export class ProviderSettingsManager { } } - public async import(providerProfiles: ProviderProfiles) { + public async import(providerProfiles: ProviderProfiles, onPersisted?: () => void) { try { - return await this.lock(() => this.store(providerProfiles)) + return await this.lock(async () => { + await this.store(providerProfiles) + // Publish local choices before a later writer can acquire the storage lock. + onPersisted?.() + }) } catch (error) { throw new Error(`Failed to import provider profiles: ${error}`) } diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index c77464f9f2..66f42b9b40 100644 --- a/src/core/config/__tests__/ProviderSettingsManager.spec.ts +++ b/src/core/config/__tests__/ProviderSettingsManager.spec.ts @@ -89,6 +89,36 @@ describe("ProviderSettingsManager", () => { providerSettingsManager = new ProviderSettingsManager(mockContext) }) + it.each([ + { operation: "import", fails: false }, + { operation: "import", fails: true }, + { operation: "mode write", fails: false }, + { operation: "mode write", fails: true }, + ])("publishes $operation choices only after persistence (failure: $fails)", async ({ operation, fails }) => { + let release = () => {} + const pending = new Promise((resolve) => { + release = resolve + }) + mockSecrets.store.mockImplementationOnce(async () => { + await pending + if (fails) throw new Error("write failed") + }) + const published = vi.fn() + const write = + operation === "import" + ? providerSettingsManager.import( + { currentApiConfigName: "default", apiConfigs: {}, modeApiConfigs: { code: "imported-id" } }, + published, + ) + : providerSettingsManager.setModeConfig("code", "chosen-id", published) + await vi.waitFor(() => expect(mockSecrets.store).toHaveBeenCalledOnce()) + expect(published).not.toHaveBeenCalled() + const result = fails ? expect(write).rejects.toThrow("write failed") : expect(write).resolves.toBeUndefined() + release() + await result + expect(published).toHaveBeenCalledTimes(fails ? 0 : 1) + }) + describe("initialize", () => { it("should not write to storage when secrets.get returns null", async () => { // Mock readConfig to return null diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index de654db60c..2e840b7ab2 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -187,7 +187,9 @@ describe("importExport", () => { beforeEach(() => { ;(fs.readFile as Mock).mockResolvedValue(validFileContent) mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles) - mockProviderSettingsManager.import.mockResolvedValue(undefined) + mockProviderSettingsManager.import.mockImplementation(async (_profiles, onPersisted) => { + onPersisted?.() + }) mockProviderSettingsManager.listConfig.mockResolvedValue(profileMetadata) mockContextProxy.setValues.mockResolvedValue(undefined) mockContextProxy.setValue.mockResolvedValue(undefined) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index df9281804d..0dca4c1a58 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -229,8 +229,13 @@ export async function importSettingsFromPath( // OpenAI Compatible settings are now correctly stored in codebaseIndexConfig // They will be imported automatically with the config - no special handling needed - await providerSettingsManager.import(providerProfiles) - onModeConfigIdsImported?.(rawProviderProfiles.modeApiConfigs ?? {}) + if (onModeConfigIdsImported) { + await providerSettingsManager.import(providerProfiles, () => + onModeConfigIdsImported(rawProviderProfiles.modeApiConfigs ?? {}), + ) + } else { + await providerSettingsManager.import(providerProfiles) + } await contextProxy.setValues(sanitizedGlobalSettings) // Set the current provider. diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3d46f26e02..5b1c3a6e5a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2115,7 +2115,8 @@ export class ClineProvider if (signal?.aborted) return // Load the saved API config for the new mode if it exists. - const savedConfigId = await this.getModeConfigId(newMode) + const savedConfigId = await this.getModeConfigId(newMode, signal) + if (signal?.aborted) return const listApiConfig = await this.providerSettingsManager.listConfig() if (signal?.aborted) return @@ -2183,23 +2184,26 @@ export class ClineProvider * id whose profile no longer exists (e.g. after a reset) is dropped in favour of the shared map, * while an unreadable opening snapshot keeps this window’s current profile. */ - private async getModeConfigId(mode: Mode): Promise { + private async getModeConfigId(mode: Mode, signal?: AbortSignal): Promise { + if (signal?.aborted) return undefined const localId = this.localModeConfigIds.get(mode) const initialIds = localId === undefined ? await this.initialModeConfigIds : undefined - if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode) + if (signal?.aborted) return undefined + if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode, signal) const cachedId = localId ?? initialIds?.[mode] if (cachedId === undefined) return undefined const profiles = await this.providerSettingsManager.listConfig() + if (signal?.aborted) return undefined // A profile selection may have changed while either asynchronous read was pending. - if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode) + if (this.localModeConfigIds.get(mode) !== localId) return this.getModeConfigId(mode, signal) if (profiles.some(({ id }) => id === cachedId)) return cachedId if (localId !== undefined) this.localModeConfigIds.delete(mode) return this.providerSettingsManager.getModeConfigId(mode) } - /** Adopt only mappings explicitly written by an import in this window. */ + /** Adopt imported mappings from the manager's persistence callback, while its storage lock is held. */ public adoptModeConfigIds(ids: Record): void { for (const [mode, id] of Object.entries(ids)) { this.localModeConfigIds.set(mode, id) @@ -2225,7 +2229,10 @@ export class ClineProvider const previousId = this.localModeConfigIds.get(mode) this.localModeConfigIds.set(mode, id) try { - await this.providerSettingsManager.setModeConfig(mode, id) + await this.providerSettingsManager.setModeConfig(mode, id, () => { + // An import may have adopted another choice while this write waited for the storage lock. + this.localModeConfigIds.set(mode, id) + }) } catch (error) { // Keep this window consistent with the shared map the failed write left unchanged. if (this.localModeConfigIds.get(mode) === id) { diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index a11048d7fb..0f7e53db4c 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -28,6 +28,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { defaultModeSlug } from "../../../shared/modes" import { experimentDefault } from "../../../shared/experiments" import { setTtsEnabled } from "../../../utils/tts" +import { ProviderSettingsManager } from "../../config/ProviderSettingsManager" import { ContextProxy } from "../../config/ContextProxy" import { WorkspaceIndexingEnablementManager } from "../../../services/code-index/workspace-indexing-enablement-manager" import { Task, TaskOptions } from "../../task/Task" @@ -38,7 +39,12 @@ import { webviewMessageHandler } from "../webviewMessageHandler" import { Terminal } from "../../../integrations/terminal/Terminal" import { MessageManager } from "../../message-manager" import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../../api/providers/fetchers/lmstudio" -import { makeCompositeDisposable, makeEventEmitter } from "../../../test-utils/vscode" +import { + makeCompositeDisposable, + makeEventEmitter, + makeUri, + makeWorkspaceConfiguration, +} from "../../../test-utils/vscode" import { WebviewFocusTracker } from "../WebviewFocusTracker" import { resolveChatProvider } from "../../../activate/resolveChatProvider" @@ -4487,6 +4493,135 @@ describe("ClineProvider", () => { .spyOn(provider.providerSettingsManager, "listConfig") .mockResolvedValue(ids.map((id) => ({ name: `${id}-name`, id }))) + it.each([false, true])("captures opening choices in the constructor (read failure: %s)", async (fails) => { + const snapshot = vi.spyOn(ProviderSettingsManager.prototype, "getModeConfigIds") + if (fails) snapshot.mockRejectedValueOnce(new Error("unreadable")) + else snapshot.mockResolvedValueOnce({ code: "opened-id" }) + const opened = new ClineProvider( + mockContext, + mockOutputChannel, + "sidebar", + new ContextProxy(mockContext), + new WebviewFocusTracker(), + ) + try { + expect(snapshot).toHaveBeenCalledOnce() + vi.spyOn(opened.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "opened", id: "opened-id" }, + ]) + const sharedLookup = vi + .spyOn(opened.providerSettingsManager, "getModeConfigId") + .mockResolvedValue("other-window-id") + snapshot.mockResolvedValue({ code: "other-window-id" }) + await expect(opened["getModeConfigId"]("code")).resolves.toBe(fails ? undefined : "opened-id") + expect(sharedLookup).not.toHaveBeenCalled() + } finally { + await opened.dispose() + snapshot.mockRestore() + } + }) + + it.each(["snapshot", "profiles"])("stops a cancelled mode switch during %s lookup", async (stage) => { + let release = () => {} + const pending = new Promise((resolve) => { + release = resolve + }) + provider["initialModeConfigIds"] = + stage === "snapshot" + ? pending.then(() => ({ code: "opened-id" })) + : Promise.resolve({ code: "opened-id" }) + const list = vi.spyOn(provider.providerSettingsManager, "listConfig").mockImplementation(async () => { + if (stage === "profiles") await pending + return [{ name: "opened", id: "opened-id" }] + }) + const lookup = vi.fn(provider["getModeConfigId"].bind(provider)) + provider["getModeConfigId"] = lookup + const controller = new AbortController() + const switched = provider["handleModeSwitchUnlocked"]("code", null, controller.signal) + await vi.waitFor(() => expect(lookup).toHaveBeenCalledOnce()) + if (stage === "profiles") await vi.waitFor(() => expect(list).toHaveBeenCalledOnce()) + controller.abort() + release() + await switched + expect(list).toHaveBeenCalledTimes(stage === "snapshot" ? 0 : 1) + expect(provider["localModeConfigIds"].has("code")).toBe(false) + }) + + it.each(["manual", "automatic"])("adopts imported choices through the %s import entry point", async (entry) => { + const filePath = "/settings.json" + const fsPromises = (await import("fs/promises")).default + const originalReadFile = vi.mocked(fsPromises.readFile).getMockImplementation()! + vi.mocked(fsPromises.readFile).mockResolvedValue( + JSON.stringify({ + providerProfiles: { + currentApiConfigName: "imported", + apiConfigs: { imported: { id: "imported-id", apiProvider: providerIdentifiers.anthropic } }, + modeApiConfigs: { code: "imported-id" }, + }, + globalSettings: {}, + }), + ) + vi.spyOn(provider, "postStateToWebview").mockResolvedValue(undefined) + const options = { + providerSettingsManager: provider.providerSettingsManager, + contextProxy: provider.contextProxy, + customModesManager: provider.customModesManager, + provider, + } + if (entry === "manual") { + vi.mocked(vscode.window.showOpenDialog).mockResolvedValueOnce([makeUri(filePath)]) + const { importSettingsWithFeedback } = await import("../../config/importExport") + await importSettingsWithFeedback(options) + } else { + const fsUtils = await import("../../../utils/fs") + const exists = vi.spyOn(fsUtils, "fileExistsAtPath").mockResolvedValue(true) + const originalGetConfiguration = vi.mocked(vscode.workspace.getConfiguration).getMockImplementation()! + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue( + makeWorkspaceConfiguration({ autoImportSettingsPath: filePath }), + ) + const { autoImportSettings } = await import("../../../utils/autoImportSettings") + await autoImportSettings(mockOutputChannel, { + ...options, + onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids), + }) + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("Successfully imported settings"), + ) + exists.mockRestore() + vi.mocked(vscode.workspace.getConfiguration).mockImplementation(originalGetConfiguration) + } + await expect(provider["getModeConfigId"]("code")).resolves.toBe("imported-id") + await expect(provider.providerSettingsManager.getModeConfigId("code")).resolves.toBe("imported-id") + vi.mocked(fsPromises.readFile).mockImplementation(originalReadFile) + }) + + it.each(["import-first", "choice-first"])("keeps local choices in persisted order (%s)", async (order) => { + const manager = provider.providerSettingsManager + const profiles = { + currentApiConfigName: "imported", + apiConfigs: { imported: { id: "imported-id" }, chosen: { id: "chosen-id" } }, + modeApiConfigs: { code: "imported-id" }, + } + let release = () => {} + const pending = new Promise((resolve) => { + release = resolve + }) + const originalStore = vi.mocked(mockContext.secrets.store).getMockImplementation()! + vi.spyOn(mockContext.secrets, "store").mockImplementationOnce(async (key, value) => { + await pending + await originalStore(key, value) + }) + const imported = () => manager.import(profiles, () => provider.adoptModeConfigIds(profiles.modeApiConfigs)) + const chosen = () => provider.setModeConfig("code", "chosen-id") + const first = order === "import-first" ? imported() : chosen() + const second = order === "import-first" ? chosen() : imported() + release() + await Promise.all([first, second]) + const expected = order === "import-first" ? "chosen-id" : "imported-id" + await expect(manager.getModeConfigId("code")).resolves.toBe(expected) + await expect(provider["getModeConfigId"]("code")).resolves.toBe(expected) + }) + it("prefers this window's mode profile over one another window saved in shared storage", async () => { const manager = provider.providerSettingsManager existingProfiles("this-window-id", "other-window-id") @@ -4495,7 +4630,7 @@ describe("ClineProvider", () => { await provider["setModeConfig"]("code", "this-window-id") - expect(manager.setModeConfig).toHaveBeenCalledWith("code", "this-window-id") + expect(manager.setModeConfig).toHaveBeenCalledWith("code", "this-window-id", expect.any(Function)) await expect(provider["getModeConfigId"]("code")).resolves.toBe("this-window-id") }) @@ -4653,7 +4788,7 @@ describe("ClineProvider", () => { rejectWrite(new Error("write failed")) await failure await succeeded - expect(sharedWrite).toHaveBeenLastCalledWith("code", nextId) + expect(sharedWrite).toHaveBeenLastCalledWith("code", nextId, expect.any(Function)) await expect(provider["getModeConfigId"]("code")).resolves.toBe(nextId) }, ) @@ -4698,7 +4833,11 @@ describe("ClineProvider", () => { await messageHandler({ type: "mode", text: "architect" }) // Should save current config as default for architect mode - expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "current-id") + expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith( + "architect", + "current-id", + expect.any(Function), + ) }) it("saves config as default for current mode when loading config", async () => { @@ -4725,7 +4864,11 @@ describe("ClineProvider", () => { await messageHandler({ type: "loadApiConfiguration", text: "new-config" }) // Should save new config as default for architect mode - expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "new-id") + expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith( + "architect", + "new-id", + expect.any(Function), + ) }) it("load API configuration by ID works and updates mode config", async () => { @@ -4752,7 +4895,11 @@ describe("ClineProvider", () => { await messageHandler({ type: "loadApiConfigurationById", text: "config-id-123" }) // Should save new config as default for architect mode - expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "config-id-123") + expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith( + "architect", + "config-id-123", + expect.any(Function), + ) // Ensure the `activateProfile` method was called with the correct ID expect(provider.providerSettingsManager.activateProfile).toHaveBeenCalledWith({ id: "config-id-123" }) @@ -4932,7 +5079,11 @@ describe("ClineProvider", () => { }) // Should save config as default for current mode - expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("code", "test-id") + expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith( + "code", + "test-id", + expect.any(Function), + ) }) test("file content includes line numbers", async () => { @@ -5314,7 +5465,11 @@ describe("ClineProvider", () => { expect(mockContext.globalState.update).toHaveBeenCalledWith("mode", "architect") // Verify current config was saved as default for new mode - expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith("architect", "current-id") + expect(provider.providerSettingsManager.setModeConfig).toHaveBeenCalledWith( + "architect", + "current-id", + expect.any(Function), + ) // Verify state was posted to webview expect(mockPostMessage).toHaveBeenCalledWith(expect.objectContaining({ type: "state" })) diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 11a3fafeaf..e508fbd2de 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -456,7 +456,11 @@ describe("ClineProvider - Sticky Provider Profile", () => { ) if (persistModeConfig) { - expect(setModeConfigSpy).toHaveBeenCalledExactlyOnceWith("architect", "new-profile-id") + expect(setModeConfigSpy).toHaveBeenCalledExactlyOnceWith( + "architect", + "new-profile-id", + expect.any(Function), + ) } else { expect(setModeConfigSpy).not.toHaveBeenCalled() } diff --git a/src/extension/__tests__/api-configuration.spec.ts b/src/extension/__tests__/api-configuration.spec.ts index 31b0be733c..b093eb2be2 100644 --- a/src/extension/__tests__/api-configuration.spec.ts +++ b/src/extension/__tests__/api-configuration.spec.ts @@ -52,8 +52,8 @@ describe("API - configuration", () => { }), ) expect(setModeConfig).toHaveBeenCalledTimes(2) - expect(setModeConfig).toHaveBeenCalledWith("code", "code-config") - expect(setModeConfig).toHaveBeenCalledWith("new-custom-mode", "architect-config") + expect(setModeConfig).toHaveBeenCalledWith("code", "code-config", expect.any(Function)) + expect(setModeConfig).toHaveBeenCalledWith("new-custom-mode", "architect-config", expect.any(Function)) await expect(provider["getModeConfigId"]("code")).resolves.toBe("code-config") await expect(provider["getModeConfigId"]("new-custom-mode")).resolves.toBe("architect-config") expect(postStateToWebview).toHaveBeenCalledOnce()