Sfoglia il codice sorgente

fix: Apply updated API profile settings when provider/model unchanged (#9208) (#9210)

fix: apply updated API profile settings when provider/model unchanged (#9208)
Hannes Rudolph 1 mese fa
parent
commit
8902facf49

+ 15 - 8
src/core/webview/ClineProvider.ts

@@ -1301,15 +1301,23 @@ export class ClineProvider
 	// Provider Profile Management
 
 	/**
-	 * Updates the current task's API handler if the provider or model has changed.
-	 * Also synchronizes the task.apiConfiguration so subsequent comparisons and logic
-	 * (protocol selection, reasoning display, model metadata) use the latest profile.
+	 * Updates the current task's API handler.
+	 * Rebuilds when:
+	 * - provider or model changes, OR
+	 * - explicitly forced (e.g., user-initiated profile switch/save to apply changed settings like headers/baseUrl/tier).
+	 * Always synchronizes task.apiConfiguration with latest provider settings.
 	 * @param providerSettings The new provider settings to apply
+	 * @param options.forceRebuild Force rebuilding the API handler regardless of provider/model equality
 	 */
-	private updateTaskApiHandlerIfNeeded(providerSettings: ProviderSettings): void {
+	private updateTaskApiHandlerIfNeeded(
+		providerSettings: ProviderSettings,
+		options: { forceRebuild?: boolean } = {},
+	): void {
 		const task = this.getCurrentTask()
 		if (!task) return
 
+		const { forceRebuild = false } = options
+
 		// Determine if we need to rebuild using the previous configuration snapshot
 		const prevConfig = task.apiConfiguration
 		const prevProvider = prevConfig?.apiProvider
@@ -1317,7 +1325,7 @@ export class ClineProvider
 		const newProvider = providerSettings.apiProvider
 		const newModelId = getModelId(providerSettings)
 
-		if (prevProvider !== newProvider || prevModelId !== newModelId) {
+		if (forceRebuild || prevProvider !== newProvider || prevModelId !== newModelId) {
 			task.api = buildApiHandler(providerSettings)
 		}
 
@@ -1373,7 +1381,7 @@ export class ClineProvider
 
 				// Change the provider for the current task.
 				// TODO: We should rename `buildApiHandler` for clarity (e.g. `getProviderClient`).
-				this.updateTaskApiHandlerIfNeeded(providerSettings)
+				this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
 			} else {
 				await this.updateGlobalState("listApiConfigMeta", await this.providerSettingsManager.listConfig())
 			}
@@ -1428,9 +1436,8 @@ export class ClineProvider
 		if (id) {
 			await this.providerSettingsManager.setModeConfig(mode, id)
 		}
-
 		// Change the provider for the current task.
-		this.updateTaskApiHandlerIfNeeded(providerSettings)
+		this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true })
 
 		await this.postStateToWebview()
 

+ 20 - 10
src/core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts

@@ -250,7 +250,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 	})
 
 	describe("upsertProviderProfile", () => {
-		test("does NOT rebuild API handler when provider and model unchanged, but task.apiConfiguration is synced", async () => {
+		test("rebuilds API handler when provider/model unchanged but profile settings changed (explicit save)", async () => {
 			// Create a task with the current config
 			const mockTask = new Task({
 				...defaultTaskOptions,
@@ -285,10 +285,15 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 				true,
 			)
 
-			// Verify buildApiHandler was NOT called since provider/model unchanged
-			expect(buildApiHandlerMock).not.toHaveBeenCalled()
-			// Verify the task's api property was NOT reassigned (still same reference)
-			expect(mockTask.api).toBe(originalApi)
+			// Verify buildApiHandler WAS called because we force rebuild on explicit save/switch
+			expect(buildApiHandlerMock).toHaveBeenCalledWith(
+				expect.objectContaining({
+					apiProvider: "openrouter",
+					openRouterModelId: "openai/gpt-4",
+				}),
+			)
+			// Verify the task's api property was reassigned (new client)
+			expect(mockTask.api).not.toBe(originalApi)
 			// Verify task.apiConfiguration was synchronized with non-model fields
 			expect((mockTask as any).apiConfiguration.openRouterModelId).toBe("openai/gpt-4")
 			expect((mockTask as any).apiConfiguration.rateLimitSeconds).toBe(5)
@@ -390,7 +395,7 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 	})
 
 	describe("activateProviderProfile", () => {
-		test("does NOT rebuild API handler when provider and model unchanged, but task.apiConfiguration is synced", async () => {
+		test("rebuilds API handler when provider/model unchanged but settings differ (explicit profile switch)", async () => {
 			const mockTask = new Task({
 				...defaultTaskOptions,
 				apiConfiguration: {
@@ -423,10 +428,15 @@ describe("ClineProvider - API Handler Rebuild Guard", () => {
 
 			await provider.activateProviderProfile({ name: "test-config" })
 
-			// Verify buildApiHandler was NOT called
-			expect(buildApiHandlerMock).not.toHaveBeenCalled()
-			// Verify the API reference wasn't changed
-			expect(mockTask.api).toBe(originalApi)
+			// Verify buildApiHandler WAS called due to forced rebuild on explicit switch
+			expect(buildApiHandlerMock).toHaveBeenCalledWith(
+				expect.objectContaining({
+					apiProvider: "openrouter",
+					openRouterModelId: "openai/gpt-4",
+				}),
+			)
+			// Verify the API reference changed
+			expect(mockTask.api).not.toBe(originalApi)
 			// Verify task.apiConfiguration was synchronized
 			expect((mockTask as any).apiConfiguration.openRouterModelId).toBe("openai/gpt-4")
 			expect((mockTask as any).apiConfiguration.modelTemperature).toBe(0.9)