fix(provider): preserve queued profile mutation ordering

Author: edelauna

src/core/webview/ClineProvider.ts

@@ -195,7 +195,7 @@ export class ClineProvider
 	private taskHistoryStoreInitialized = false
 	private globalStateWriteThroughTimer: ReturnType<typeof setTimeout> | null = null
 	private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds
-	private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
+	public static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds
 	private providerProfileMutationQueue = Promise.resolve()
 
 	private runDelegationTransition<T>(parentTaskId: string, fn: () => Promise<T>): Promise<T> {
@@ -204,11 +204,12 @@ export class ClineProvider
 	}
 
 	private enqueueProviderProfileMutation<T>(fn: () => Promise<T>): Promise<T> {
+		// Run after either outcome so a rejected mutation never poisons the queue.
 		const run = this.providerProfileMutationQueue.then(fn, fn)
 		let timedOut = false
 		const callerResult = this.withProviderProfileMutationTimeout(run, () => {
 			timedOut = true
-			this.log("Provider profile mutation timed out; releasing the mutation queue")
+			this.log("Provider profile mutation timed out; waiting for the in-flight mutation to settle")
 		})
 
 		void run.then(
@@ -228,9 +229,9 @@ export class ClineProvider
 			},
 		)
 
-		// Advance from the timeout-bounded caller result, rather than the raw operation. A
-		// provider call that never settles must not block all subsequent profile changes.
-		this.providerProfileMutationQueue = callerResult.then(
+		// Keep the raw operation as the queue boundary. Releasing the queue on timeout
+		// would allow its later state writes to overwrite a subsequent mutation.
+		this.providerProfileMutationQueue = run.then(
 			() => undefined,
 			() => undefined,
 		)
@@ -252,6 +253,7 @@ export class ClineProvider
 			}
 		})
 	}
+
 	private readonly pendingEditOperations: PendingEditOperationStore
 
 	private cloudOrganizationsCache: CloudOrganizationMembership[] | null = null
@@ -1560,10 +1562,8 @@ export class ClineProvider
 	 * @param targetTask The task whose in-memory mode should be updated. Defaults to the
 	 * current task. Pass null to apply only global mode/profile effects for a pending child.
 	 */
-	public async handleModeSwitch(newMode: Mode, targetTask?: Task | null) {
-		return this.enqueueProviderProfileMutation(() =>
-			this.handleModeSwitchUnlocked(newMode, targetTask === undefined ? this.getCurrentTask() : targetTask),
-		)
+	public async handleModeSwitch(newMode: Mode, targetTask: Task | null | undefined = this.getCurrentTask()) {
+		return this.enqueueProviderProfileMutation(() => this.handleModeSwitchUnlocked(newMode, targetTask))
 	}
 
 	private async handleModeSwitchUnlocked(newMode: Mode, targetTask: Task | null | undefined): Promise<void> {

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

@@ -457,6 +457,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 
 		test("provider profile mutation rejection does not poison later queued mutations", async () => {
 			const firstError = new Error("first profile failed")
+			const setValueSpy = vi.spyOn(provider.contextProxy, "setValue")
 
 			provider["providerSettingsManager"].activateProfile = vi
 				.fn()
@@ -470,14 +471,27 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 
 			await expect(provider.activateProviderProfile({ name: "first-profile" })).rejects.toThrow(firstError)
 			await expect(provider.activateProviderProfile({ name: "second-profile" })).resolves.toBeUndefined()
+			expect(setValueSpy).toHaveBeenCalledWith("currentApiConfigName", "second-profile")
 		})
 
-		test("provider profile mutation timeout releases later queued mutations", async () => {
+		test("timed-out provider profile mutations retain the queue boundary until they settle", async () => {
 			vi.useFakeTimers()
 			const logSpy = vi.spyOn(provider, "log")
+			let resolveFirst!: () => void
+			const firstActivation = new Promise<void>((resolve) => {
+				resolveFirst = resolve
+			})
 			provider["providerSettingsManager"].activateProfile = vi
 				.fn()
-				.mockImplementationOnce(() => new Promise<never>(() => {}))
+				.mockImplementationOnce(async () => {
+					await firstActivation
+					return {
+						name: "first-profile",
+						id: "first-id",
+						apiProvider: "openrouter",
+						openRouterModelId: "openai/gpt-4",
+					}
+				})
 				.mockResolvedValueOnce({
 					name: "second-profile",
 					id: "second-id",
@@ -488,17 +502,24 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 			try {
 				const first = provider.activateProviderProfile({ name: "first-profile" })
 				const firstResult = expect(first).rejects.toThrow("Provider profile mutation timed out")
-				await vi.advanceTimersByTimeAsync(30_000)
+				await vi.advanceTimersByTimeAsync(ClineProvider.PENDING_OPERATION_TIMEOUT_MS)
 				await firstResult
 
-				await expect(provider.activateProviderProfile({ name: "second-profile" })).resolves.toBeUndefined()
-				expect(logSpy).toHaveBeenCalledWith("Provider profile mutation timed out; releasing the mutation queue")
+				const second = provider.activateProviderProfile({ name: "second-profile" })
+				expect(provider["providerSettingsManager"].activateProfile).toHaveBeenCalledTimes(1)
+
+				resolveFirst()
+				await expect(second).resolves.toBeUndefined()
+				expect(provider["providerSettingsManager"].activateProfile).toHaveBeenCalledTimes(2)
+				expect(logSpy).toHaveBeenCalledWith(
+					"Provider profile mutation timed out; waiting for the in-flight mutation to settle",
+				)
 			} finally {
 				vi.useRealTimers()
 			}
 		})
 
-		test("mode switch resolves its default task when its queued mutation starts", async () => {
+		test("mode switch preserves its default task when queued behind a profile mutation", async () => {
 			let releaseProfileActivation!: () => void
 			const profileActivation = new Promise<void>((resolve) => {
 				releaseProfileActivation = resolve
@@ -515,6 +536,10 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 
 			const firstTask = new Task(defaultTaskOptions)
 			const secondTask = new Task(defaultTaskOptions)
+			Object.defineProperty(firstTask, "taskId", { value: "first-task-id" })
+			Object.defineProperty(secondTask, "taskId", { value: "second-task-id" })
+			firstTask["_taskMode"] = "code" as Mode
+			secondTask["_taskMode"] = "code" as Mode
 			await provider.addClineToStack(firstTask)
 
 			const profileSwitch = provider.activateProviderProfile({ name: "first-profile" })
@@ -525,8 +550,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 			await profileSwitch
 			await modeSwitch
 
-			expect(firstTask["_taskMode"]).not.toBe("ask")
-			expect(secondTask["_taskMode"]).toBe("ask")
+			expect(firstTask["_taskMode"]).toBe("ask")
+			expect(secondTask["_taskMode"]).toBe("code")
 		})
 
 		test("fan-out preparation leaves the focused task untouched", async () => {
@@ -570,6 +595,8 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 			expect(postStateSpy).not.toHaveBeenCalled()
 			expect(setValueSpy).not.toHaveBeenCalledWith("currentApiConfigName", "ask-profile")
 			expect(setProviderSettingsSpy).not.toHaveBeenCalled()
+			expect(emitSpy).toHaveBeenCalledWith(RooCodeEventName.ModeChanged, "ask")
+			expect(provider["providerSettingsManager"].activateProfile).toHaveBeenCalledWith({ name: "ask-profile" })
 		})
 
 		test("calls updateApiConfiguration when provider/model unchanged but settings differ (explicit profile switch)", async () => {

src/extension/__tests__/api-configuration.spec.ts

@@ -30,9 +30,36 @@ describe("API - configuration", () => {
 		})
 
 		expect(saveConfig).toHaveBeenCalledWith("default", expect.objectContaining({ currentApiConfigName: "default" }))
+		expect(setValues).toHaveBeenCalledWith(
+			expect.objectContaining({
+				currentApiConfigName: "default",
+				modeApiConfigs: expect.anything(),
+			}),
+		)
 		expect(setModeConfig).toHaveBeenCalledTimes(2)
 		expect(setModeConfig).toHaveBeenCalledWith("code", "code-config")
 		expect(setModeConfig).toHaveBeenCalledWith("architect", "architect-config")
 		expect(postStateToWebview).toHaveBeenCalledOnce()
 	})
+
+	it("does not persist mode mappings when none are supplied", async () => {
+		const setValues = vi.fn().mockResolvedValue(undefined)
+		const saveConfig = vi.fn().mockResolvedValue("default-id")
+		const setModeConfig = vi.fn().mockResolvedValue(undefined)
+		const postStateToWebview = vi.fn().mockResolvedValue(undefined)
+		const provider = {
+			context: {},
+			on: vi.fn(),
+			contextProxy: { setValues },
+			providerSettingsManager: { saveConfig, setModeConfig },
+			postStateToWebview,
+		} as unknown as ClineProvider
+		const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel
+		const api = new API(outputChannel, provider)
+
+		await api.setConfiguration({ currentApiConfigName: "default" })
+
+		expect(setModeConfig).not.toHaveBeenCalled()
+		expect(postStateToWebview).toHaveBeenCalledOnce()
+	})
 })