Skip to content

Commit c513df5

Browse files
fix: merge settings and versionedSettings for Roo provider models (#10030)
1 parent 21c2d93 commit c513df5

File tree

2 files changed

+68
-17
lines changed

2 files changed

+68
-17
lines changed

src/api/providers/fetchers/__tests__/roo.spec.ts

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ describe("getRooModels", () => {
803803
expect(model.nestedConfig).toEqual({ key: "value" })
804804
})
805805

806-
it("should apply versioned settings when version matches", async () => {
806+
it("should apply versioned settings when version matches, overriding plain settings", async () => {
807807
const mockResponse = {
808808
object: "list",
809809
data: [
@@ -845,11 +845,65 @@ describe("getRooModels", () => {
845845

846846
const models = await getRooModels(baseUrl, apiKey)
847847

848-
// Versioned settings should be used instead of plain settings
848+
// Versioned settings should override the same properties from plain settings
849849
expect(models["test/versioned-model"].includedTools).toEqual(["apply_patch", "search_replace"])
850850
expect(models["test/versioned-model"].excludedTools).toEqual(["apply_diff", "write_to_file"])
851851
})
852852

853+
it("should merge settings and versionedSettings, with versioned settings taking precedence", async () => {
854+
const mockResponse = {
855+
object: "list",
856+
data: [
857+
{
858+
id: "test/merged-settings-model",
859+
object: "model",
860+
created: 1234567890,
861+
owned_by: "test",
862+
name: "Model with Merged Settings",
863+
description: "Model with both settings and versionedSettings that should be merged",
864+
context_window: 128000,
865+
max_tokens: 8192,
866+
type: "language",
867+
tags: ["tool-use"],
868+
pricing: {
869+
input: "0.0001",
870+
output: "0.0002",
871+
},
872+
// Plain settings - provides base configuration
873+
settings: {
874+
includedTools: ["apply_patch"],
875+
excludedTools: ["apply_diff", "write_to_file"],
876+
reasoningEffort: "medium",
877+
},
878+
// Versioned settings - adds version-specific overrides
879+
versionedSettings: {
880+
"1.0.0": {
881+
supportsTemperature: false,
882+
supportsReasoningEffort: ["none", "low", "medium", "high"],
883+
},
884+
},
885+
},
886+
],
887+
}
888+
889+
mockFetch.mockResolvedValueOnce({
890+
ok: true,
891+
json: async () => mockResponse,
892+
})
893+
894+
const models = await getRooModels(baseUrl, apiKey)
895+
const model = models["test/merged-settings-model"] as Record<string, unknown>
896+
897+
// Properties from plain settings should be present
898+
expect(model.includedTools).toEqual(["apply_patch"])
899+
expect(model.excludedTools).toEqual(["apply_diff", "write_to_file"])
900+
expect(model.reasoningEffort).toBe("medium")
901+
902+
// Properties from versioned settings should also be present
903+
expect(model.supportsTemperature).toBe(false)
904+
expect(model.supportsReasoningEffort).toEqual(["none", "low", "medium", "high"])
905+
})
906+
853907
it("should use plain settings when no versioned settings version matches", async () => {
854908
const mockResponse = {
855909
object: "list",

src/api/providers/fetchers/roo.ts

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -130,33 +130,30 @@ export async function getRooModels(baseUrl: string, apiKey?: string): Promise<Mo
130130
// Settings allow the proxy to dynamically configure model-specific options
131131
// like includedTools, excludedTools, reasoningEffort, etc.
132132
//
133-
// Two fields are used for backward compatibility:
134-
// - `settings`: Plain values that work with all client versions (e.g., { includedTools: ['search_replace'] })
135-
// - `versionedSettings`: Version-keyed settings (e.g., { '3.36.4': { includedTools: ['search_replace'] } })
133+
// Two fields are used together:
134+
// - `settings`: Base values that work with all client versions (e.g., { includedTools: ['search_replace'] })
135+
// - `versionedSettings`: Version-keyed overrides (e.g., { '3.36.4': { supportsTemperature: false } })
136136
//
137-
// New clients check versionedSettings first - if a matching version is found, those settings are used.
138-
// Otherwise, falls back to plain `settings`. Old clients only see `settings`.
137+
// Settings are merged: `settings` provides the base, `versionedSettings` layers on top.
138+
// This allows the proxy to set common configuration in `settings` and version-specific
139+
// overrides in `versionedSettings`. Old clients only see `settings`.
139140
const apiSettings = model.settings as Record<string, unknown> | undefined
140141
const apiVersionedSettings = model.versionedSettings as VersionedSettings | undefined
141142

142143
// Start with base model info
143144
let modelInfo: ModelInfo = { ...baseModelInfo }
144145

145-
// Try to resolve versioned settings first (finds highest version <= current plugin version)
146-
// If versioned settings match, use them exclusively (they contain all necessary settings)
147-
// Otherwise fall back to plain settings for backward compatibility
146+
// Apply plain settings first as the base configuration
147+
if (apiSettings) {
148+
modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
149+
}
150+
151+
// Then layer versioned settings on top (can override plain settings)
148152
if (apiVersionedSettings) {
149153
const resolvedVersionedSettings = resolveVersionedSettings<Partial<ModelInfo>>(apiVersionedSettings)
150154
if (Object.keys(resolvedVersionedSettings).length > 0) {
151-
// Versioned settings found - use them exclusively
152155
modelInfo = { ...modelInfo, ...resolvedVersionedSettings }
153-
} else if (apiSettings) {
154-
// No matching versioned settings - fall back to plain settings
155-
modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
156156
}
157-
} else if (apiSettings) {
158-
// No versioned settings at all - use plain settings
159-
modelInfo = { ...modelInfo, ...(apiSettings as Partial<ModelInfo>) }
160157
}
161158

162159
models[modelId] = modelInfo

0 commit comments

Comments
 (0)