Skip to content

Commit f28709e

Browse files
committed
fix: sanitize removed/invalid API providers to prevent infinite loop
When a user has an API provider selected that is later removed from the extension (e.g., 'glama'), the schema validation repeatedly fails causing telemetry spam and potential infinite loops. This fix: - Adds sanitizeProviderConfig() in ProviderSettingsManager to reset invalid apiProvider values during load while preserving the rest of the profile - Adds sanitizeProviderValues() in ContextProxy to prevent repeated validation errors when getProviderSettings() is called - Adds migrateInvalidApiProvider() migration to clean up invalid values from storage during initialization The profile is preserved with apiProvider reset to undefined so the user can select a new valid provider instead of losing their entire configuration.
1 parent 642a187 commit f28709e

File tree

4 files changed

+219
-9
lines changed

4 files changed

+219
-9
lines changed

src/core/config/ContextProxy.ts

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
providerSettingsSchema,
1616
globalSettingsSchema,
1717
isSecretStateKey,
18+
isProviderName,
1819
} from "@roo-code/types"
1920
import { TelemetryService } from "@roo-code/telemetry"
2021

@@ -88,9 +89,33 @@ export class ContextProxy {
8889
// Migration: Check for old nested image generation settings and migrate them
8990
await this.migrateImageGenerationSettings()
9091

92+
// Migration: Sanitize invalid/removed API providers
93+
await this.migrateInvalidApiProvider()
94+
9195
this._isInitialized = true
9296
}
9397

98+
/**
99+
* Migrates invalid/removed apiProvider values by clearing them from storage.
100+
* This handles cases where a user had a provider selected that was later removed
101+
* from the extension (e.g., "glama").
102+
*/
103+
private async migrateInvalidApiProvider() {
104+
try {
105+
const apiProvider = this.stateCache.apiProvider
106+
if (apiProvider !== undefined && !isProviderName(apiProvider)) {
107+
logger.info(`[ContextProxy] Found invalid provider "${apiProvider}" in storage - clearing it`)
108+
// Clear the invalid provider from both cache and storage
109+
this.stateCache.apiProvider = undefined
110+
await this.originalContext.globalState.update("apiProvider", undefined)
111+
}
112+
} catch (error) {
113+
logger.error(
114+
`Error during invalid API provider migration: ${error instanceof Error ? error.message : String(error)}`,
115+
)
116+
}
117+
}
118+
94119
/**
95120
* Migrates old nested openRouterImageGenerationSettings to the new flattened structure
96121
*/
@@ -266,15 +291,38 @@ export class ContextProxy {
266291
public getProviderSettings(): ProviderSettings {
267292
const values = this.getValues()
268293

294+
// Sanitize invalid/removed apiProvider values before parsing
295+
// This handles cases where a user had a provider selected that was later removed
296+
// from the extension (e.g., "glama"). We sanitize here to avoid repeated
297+
// schema validation errors that can cause infinite loops in telemetry.
298+
const sanitizedValues = this.sanitizeProviderValues(values)
299+
269300
try {
270-
return providerSettingsSchema.parse(values)
301+
return providerSettingsSchema.parse(sanitizedValues)
271302
} catch (error) {
272303
if (error instanceof ZodError) {
273304
TelemetryService.instance.captureSchemaValidationError({ schemaName: "ProviderSettings", error })
274305
}
275306

276-
return PROVIDER_SETTINGS_KEYS.reduce((acc, key) => ({ ...acc, [key]: values[key] }), {} as ProviderSettings)
307+
return PROVIDER_SETTINGS_KEYS.reduce(
308+
(acc, key) => ({ ...acc, [key]: sanitizedValues[key] }),
309+
{} as ProviderSettings,
310+
)
311+
}
312+
}
313+
314+
/**
315+
* Sanitizes provider values by resetting invalid/removed apiProvider values.
316+
* This prevents schema validation errors for removed providers.
317+
*/
318+
private sanitizeProviderValues(values: RooCodeSettings): RooCodeSettings {
319+
if (values.apiProvider !== undefined && !isProviderName(values.apiProvider)) {
320+
logger.info(`[ContextProxy] Sanitizing invalid provider "${values.apiProvider}" - resetting to undefined`)
321+
// Return a new values object without the invalid apiProvider
322+
const { apiProvider, ...restValues } = values
323+
return restValues as RooCodeSettings
277324
}
325+
return values
278326
}
279327

280328
public async setProviderSettings(values: ProviderSettings) {

src/core/config/ProviderSettingsManager.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
1212
getModelId,
1313
type ProviderName,
14+
isProviderName,
1415
} from "@roo-code/types"
1516
import { TelemetryService } from "@roo-code/telemetry"
1617

@@ -598,7 +599,10 @@ export class ProviderSettingsManager {
598599

599600
const apiConfigs = Object.entries(providerProfiles.apiConfigs).reduce(
600601
(acc, [key, apiConfig]) => {
601-
const result = providerSettingsWithIdSchema.safeParse(apiConfig)
602+
// First, sanitize invalid apiProvider values before parsing
603+
// This handles removed providers (like "glama") gracefully
604+
const sanitizedConfig = this.sanitizeProviderConfig(apiConfig)
605+
const result = providerSettingsWithIdSchema.safeParse(sanitizedConfig)
602606
return result.success ? { ...acc, [key]: result.data } : acc
603607
},
604608
{} as Record<string, ProviderSettingsWithId>,
@@ -622,6 +626,32 @@ export class ProviderSettingsManager {
622626
}
623627
}
624628

629+
/**
630+
* Sanitizes a provider config by resetting invalid/removed apiProvider values.
631+
* This handles cases where a user had a provider selected that was later removed
632+
* from the extension (e.g., "glama").
633+
*/
634+
private sanitizeProviderConfig(apiConfig: unknown): unknown {
635+
if (typeof apiConfig !== "object" || apiConfig === null) {
636+
return apiConfig
637+
}
638+
639+
const config = apiConfig as Record<string, unknown>
640+
641+
// Check if apiProvider is set and if it's still valid
642+
if (config.apiProvider !== undefined && !isProviderName(config.apiProvider)) {
643+
console.log(
644+
`[ProviderSettingsManager] Sanitizing invalid provider "${config.apiProvider}" - resetting to undefined`,
645+
)
646+
// Return a new config object without the invalid apiProvider
647+
// This effectively resets the profile so the user can select a valid provider
648+
const { apiProvider, ...restConfig } = config
649+
return restConfig
650+
}
651+
652+
return apiConfig
653+
}
654+
625655
private async store(providerProfiles: ProviderProfiles) {
626656
try {
627657
await this.context.secrets.store(this.secretsKey, JSON.stringify(providerProfiles, null, 2))

src/core/config/__tests__/ContextProxy.spec.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,4 +428,79 @@ describe("ContextProxy", () => {
428428
expect(initializeSpy).toHaveBeenCalledTimes(1)
429429
})
430430
})
431+
432+
describe("invalid apiProvider migration", () => {
433+
it("should clear invalid apiProvider from storage during initialization", async () => {
434+
// Reset and create a new proxy with invalid provider in state
435+
vi.clearAllMocks()
436+
mockGlobalState.get.mockImplementation((key: string) => {
437+
if (key === "apiProvider") {
438+
return "invalid-removed-provider" // Invalid/removed provider
439+
}
440+
return undefined
441+
})
442+
443+
const proxyWithInvalidProvider = new ContextProxy(mockContext)
444+
await proxyWithInvalidProvider.initialize()
445+
446+
// Should have cleared the invalid apiProvider
447+
expect(mockGlobalState.update).toHaveBeenCalledWith("apiProvider", undefined)
448+
})
449+
450+
it("should not modify valid apiProvider during initialization", async () => {
451+
// Reset and create a new proxy with valid provider in state
452+
vi.clearAllMocks()
453+
mockGlobalState.get.mockImplementation((key: string) => {
454+
if (key === "apiProvider") {
455+
return "anthropic" // Valid provider
456+
}
457+
return undefined
458+
})
459+
460+
const proxyWithValidProvider = new ContextProxy(mockContext)
461+
await proxyWithValidProvider.initialize()
462+
463+
// Should NOT have called update for apiProvider (it's valid)
464+
const updateCalls = mockGlobalState.update.mock.calls
465+
const apiProviderUpdateCalls = updateCalls.filter((call: any[]) => call[0] === "apiProvider")
466+
expect(apiProviderUpdateCalls.length).toBe(0)
467+
})
468+
})
469+
470+
describe("getProviderSettings", () => {
471+
it("should sanitize invalid apiProvider before parsing", async () => {
472+
// Set an invalid provider in state
473+
await proxy.updateGlobalState("apiProvider", "invalid-removed-provider" as any)
474+
await proxy.updateGlobalState("apiModelId", "some-model")
475+
476+
const settings = proxy.getProviderSettings()
477+
478+
// The invalid apiProvider should be sanitized (removed)
479+
expect(settings.apiProvider).toBeUndefined()
480+
// Other settings should still be present
481+
expect(settings.apiModelId).toBe("some-model")
482+
})
483+
484+
it("should pass through valid apiProvider", async () => {
485+
// Set a valid provider in state
486+
await proxy.updateGlobalState("apiProvider", "anthropic")
487+
await proxy.updateGlobalState("apiModelId", "claude-3-opus-20240229")
488+
489+
const settings = proxy.getProviderSettings()
490+
491+
// Valid provider should be returned
492+
expect(settings.apiProvider).toBe("anthropic")
493+
expect(settings.apiModelId).toBe("claude-3-opus-20240229")
494+
})
495+
496+
it("should handle undefined apiProvider gracefully", async () => {
497+
// Ensure no provider is set
498+
await proxy.updateGlobalState("apiProvider", undefined)
499+
500+
const settings = proxy.getProviderSettings()
501+
502+
// Should not throw and should return undefined
503+
expect(settings.apiProvider).toBeUndefined()
504+
})
505+
})
431506
})

src/core/config/__tests__/ProviderSettingsManager.spec.ts

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,55 @@ describe("ProviderSettingsManager", () => {
705705
)
706706
})
707707

708-
it("should remove invalid profiles during load", async () => {
708+
it("should sanitize invalid/removed providers by resetting apiProvider to undefined", async () => {
709+
// This tests the fix for the infinite loop issue when a provider is removed
710+
const configWithRemovedProvider = {
711+
currentApiConfigName: "valid",
712+
apiConfigs: {
713+
valid: {
714+
apiProvider: "anthropic",
715+
apiKey: "valid-key",
716+
apiModelId: "claude-3-opus-20240229",
717+
id: "valid-id",
718+
},
719+
removedProvider: {
720+
// Provider that was removed from the extension (e.g., "invalid-removed-provider")
721+
id: "removed-id",
722+
apiProvider: "invalid-removed-provider",
723+
apiKey: "some-key",
724+
apiModelId: "some-model",
725+
},
726+
},
727+
migrations: {
728+
rateLimitSecondsMigrated: true,
729+
diffSettingsMigrated: true,
730+
openAiHeadersMigrated: true,
731+
consecutiveMistakeLimitMigrated: true,
732+
todoListEnabledMigrated: true,
733+
},
734+
}
735+
736+
mockSecrets.get.mockResolvedValue(JSON.stringify(configWithRemovedProvider))
737+
738+
await providerSettingsManager.initialize()
739+
740+
const storeCalls = mockSecrets.store.mock.calls
741+
expect(storeCalls.length).toBeGreaterThan(0)
742+
const finalStoredConfigJson = storeCalls[storeCalls.length - 1][1]
743+
744+
const storedConfig = JSON.parse(finalStoredConfigJson)
745+
// The valid provider should be untouched
746+
expect(storedConfig.apiConfigs.valid).toBeDefined()
747+
expect(storedConfig.apiConfigs.valid.apiProvider).toBe("anthropic")
748+
749+
// The config with the removed provider should have its apiProvider reset to undefined
750+
// but still be present (not filtered out entirely)
751+
expect(storedConfig.apiConfigs.removedProvider).toBeDefined()
752+
expect(storedConfig.apiConfigs.removedProvider.apiProvider).toBeUndefined()
753+
expect(storedConfig.apiConfigs.removedProvider.id).toBe("removed-id")
754+
})
755+
756+
it("should sanitize invalid providers and remove non-object profiles during load", async () => {
709757
const invalidConfig = {
710758
currentApiConfigName: "valid",
711759
apiConfigs: {
@@ -715,12 +763,12 @@ describe("ProviderSettingsManager", () => {
715763
apiModelId: "claude-3-opus-20240229",
716764
rateLimitSeconds: 0,
717765
},
718-
invalid: {
719-
// Invalid API provider.
766+
invalidProvider: {
767+
// Invalid API provider - should be sanitized (kept but apiProvider reset to undefined)
720768
id: "x.ai",
721769
apiProvider: "x.ai",
722770
},
723-
// Incorrect type.
771+
// Incorrect type - should be completely removed
724772
anotherInvalid: "not an object",
725773
},
726774
migrations: {
@@ -737,10 +785,19 @@ describe("ProviderSettingsManager", () => {
737785
const finalStoredConfigJson = storeCalls[storeCalls.length - 1][1]
738786

739787
const storedConfig = JSON.parse(finalStoredConfigJson)
788+
// Valid config should be untouched
740789
expect(storedConfig.apiConfigs.valid).toBeDefined()
741-
expect(storedConfig.apiConfigs.invalid).toBeUndefined()
790+
expect(storedConfig.apiConfigs.valid.apiProvider).toBe("anthropic")
791+
792+
// Invalid provider config should be sanitized - kept but apiProvider reset to undefined
793+
expect(storedConfig.apiConfigs.invalidProvider).toBeDefined()
794+
expect(storedConfig.apiConfigs.invalidProvider.apiProvider).toBeUndefined()
795+
expect(storedConfig.apiConfigs.invalidProvider.id).toBe("x.ai")
796+
797+
// Non-object config should be completely removed
742798
expect(storedConfig.apiConfigs.anotherInvalid).toBeUndefined()
743-
expect(Object.keys(storedConfig.apiConfigs)).toEqual(["valid"])
799+
800+
expect(Object.keys(storedConfig.apiConfigs)).toEqual(["valid", "invalidProvider"])
744801
expect(storedConfig.currentApiConfigName).toBe("valid")
745802
})
746803
})

0 commit comments

Comments
 (0)