Browse Source

Revert "fix: merge settings and versionedSettings for Roo provider models" (#10034)

Chris Estreich 1 month ago
parent
commit
7766b91360
2 changed files with 17 additions and 68 deletions
  1. 2 56
      src/api/providers/fetchers/__tests__/roo.spec.ts
  2. 15 12
      src/api/providers/fetchers/roo.ts

+ 2 - 56
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, overriding plain settings", async () => {
+	it("should apply versioned settings when version matches", async () => {
 		const mockResponse = {
 			object: "list",
 			data: [
@@ -845,65 +845,11 @@ describe("getRooModels", () => {
 
 		const models = await getRooModels(baseUrl, apiKey)
 
-		// Versioned settings should override the same properties from plain settings
+		// Versioned settings should be used instead of 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",

+ 15 - 12
src/api/providers/fetchers/roo.ts

@@ -130,30 +130,33 @@ 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 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 } })
+				// 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'] } })
 				//
-				// 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`.
+				// 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`.
 				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 }
 
-				// 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)
+				// 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
 				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