From ae7d7587ed5210b40b62ae2b1127b5e7c11623d1 Mon Sep 17 00:00:00 2001
From: Hannes Rudolph <hrudolph@gmail.com>
Date: Thu, 11 Dec 2025 14:02:48 -0700
Subject: [PATCH] fix: merge settings and versionedSettings for Roo provider
 models

Previously, when a model from the Roo provider had both 'settings' and
'versionedSettings' defined, the logic treated them as mutually exclusive.
If versionedSettings matched the current version, settings was completely
ignored.

This fix changes the behavior to merge both: settings is applied first as
the base configuration, then versionedSettings is layered on top, allowing
versioned settings to override specific properties while preserving the
rest from plain settings.
---
 .../providers/fetchers/__tests__/roo.spec.ts  | 58 ++++++++++++++++++-
 src/api/providers/fetchers/roo.ts             | 27 ++++-----
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/src/api/providers/fetchers/__tests__/roo.spec.ts b/src/api/providers/fetchers/__tests__/roo.spec.ts
index cd86be0b692..6073218b121 100644
--- a/src/api/providers/fetchers/__tests__/roo.spec.ts
+++ b/src/api/providers/fetchers/__tests__/roo.spec.ts
@@ -803,7 +803,7 @@ describe("getRooModels", () => {
 		expect(model.nestedConfig).toEqual({ key: "value" })
 	})
 
-	it("should apply versioned settings when version matches", async () => {
+	it("should apply versioned settings when version matches, overriding plain settings", async () => {
 		const mockResponse = {
 			object: "list",
 			data: [
@@ -845,11 +845,65 @@ describe("getRooModels", () => {
 
 		const models = await getRooModels(baseUrl, apiKey)
 
-		// Versioned settings should be used instead of plain settings
+		// Versioned settings should override the same properties from plain settings
 		expect(models["test/versioned-model"].includedTools).toEqual(["apply_patch", "search_replace"])
 		expect(models["test/versioned-model"].excludedTools).toEqual(["apply_diff", "write_to_file"])
 	})
 
+	it("should merge settings and versionedSettings, with versioned settings taking precedence", async () => {
+		const mockResponse = {
+			object: "list",
+			data: [
+				{
+					id: "test/merged-settings-model",
+					object: "model",
+					created: 1234567890,
+					owned_by: "test",
+					name: "Model with Merged Settings",
+					description: "Model with both settings and versionedSettings that should be merged",
+					context_window: 128000,
+					max_tokens: 8192,
+					type: "language",
+					tags: ["tool-use"],
+					pricing: {
+						input: "0.0001",
+						output: "0.0002",
+					},
+					// Plain settings - provides base configuration
+					settings: {
+						includedTools: ["apply_patch"],
+						excludedTools: ["apply_diff", "write_to_file"],
+						reasoningEffort: "medium",
+					},
+					// Versioned settings - adds version-specific overrides
+					versionedSettings: {
+						"1.0.0": {
+							supportsTemperature: false,
+							supportsReasoningEffort: ["none", "low", "medium", "high"],
+						},
+					},
+				},
+			],
+		}
+
+		mockFetch.mockResolvedValueOnce({
+			ok: true,
+			json: async () => mockResponse,
+		})
+
+		const models = await getRooModels(baseUrl, apiKey)
+		const model = models["test/merged-settings-model"] as Record<string, unknown>
+
+		// Properties from plain settings should be present
+		expect(model.includedTools).toEqual(["apply_patch"])
+		expect(model.excludedTools).toEqual(["apply_diff", "write_to_file"])
+		expect(model.reasoningEffort).toBe("medium")
+
+		// Properties from versioned settings should also be present
+		expect(model.supportsTemperature).toBe(false)
+		expect(model.supportsReasoningEffort).toEqual(["none", "low", "medium", "high"])
+	})
+
 	it("should use plain settings when no versioned settings version matches", async () => {
 		const mockResponse = {
 			object: "list",
diff --git a/src/api/providers/fetchers/roo.ts b/src/api/providers/fetchers/roo.ts
index 65a2db77c39..757dda0bd05 100644
--- a/src/api/providers/fetchers/roo.ts
+++ b/src/api/providers/fetchers/roo.ts
@@ -130,33 +130,30 @@ export async function getRooModels(baseUrl: string, apiKey?: string): Promise<Mo
 				// Settings allow the proxy to dynamically configure model-specific options
 				// like includedTools, excludedTools, reasoningEffort, etc.
 				//
-				// Two fields are used for backward compatibility:
-				// - `settings`: Plain values that work with all client versions (e.g., { includedTools: ['search_replace'] })
-				// - `versionedSettings`: Version-keyed settings (e.g., { '3.36.4': { includedTools: ['search_replace'] } })
+				// Two fields are used together:
+				// - `settings`: Base values that work with all client versions (e.g., { includedTools: ['search_replace'] })
+				// - `versionedSettings`: Version-keyed overrides (e.g., { '3.36.4': { supportsTemperature: false } })
 				//
-				// New clients check versionedSettings first - if a matching version is found, those settings are used.
-				// Otherwise, falls back to plain `settings`. Old clients only see `settings`.
+				// Settings are merged: `settings` provides the base, `versionedSettings` layers on top.
+				// This allows the proxy to set common configuration in `settings` and version-specific
+				// overrides in `versionedSettings`. Old clients only see `settings`.
 				const apiSettings = model.settings as Record<string, unknown> | undefined
 				const apiVersionedSettings = model.versionedSettings as VersionedSettings | undefined
 
 				// Start with base model info
 				let modelInfo: ModelInfo = { ...baseModelInfo }
 
-				// Try to resolve versioned settings first (finds highest version <= current plugin version)
-				// If versioned settings match, use them exclusively (they contain all necessary settings)
-				// Otherwise fall back to plain settings for backward compatibility
+				// Apply plain settings first as the base configuration
+				if (apiSettings) {
+					modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
+				}
+
+				// Then layer versioned settings on top (can override plain settings)
 				if (apiVersionedSettings) {
 					const resolvedVersionedSettings = resolveVersionedSettings<Partial<ModelInfo>>(apiVersionedSettings)
 					if (Object.keys(resolvedVersionedSettings).length > 0) {
-						// Versioned settings found - use them exclusively
 						modelInfo = { ...modelInfo, ...resolvedVersionedSettings }
-					} else if (apiSettings) {
-						// No matching versioned settings - fall back to plain settings
-						modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
 					}
-				} else if (apiSettings) {
-					// No versioned settings at all - use plain settings
-					modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
 				}
 
 				models[modelId] = modelInfo