Browse Source

fix: merge settings and versionedSettings for Roo provider models (#10030)

Hannes Rudolph 1 month ago
parent
commit
c513df5ade
2 changed files with 68 additions and 17 deletions
  1. 56 2
      src/api/providers/fetchers/__tests__/roo.spec.ts
  2. 12 15
      src/api/providers/fetchers/roo.ts

+ 56 - 2
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",

+ 12 - 15
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