Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions src/core/config/ContextProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
providerSettingsSchema,
globalSettingsSchema,
isSecretStateKey,
isProviderName,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

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

// Migration: Sanitize invalid/removed API providers
await this.migrateInvalidApiProvider()

this._isInitialized = true
}

/**
* Migrates invalid/removed apiProvider values by clearing them from storage.
* This handles cases where a user had a provider selected that was later removed
* from the extension (e.g., "glama").
*/
private async migrateInvalidApiProvider() {
try {
const apiProvider = this.stateCache.apiProvider
if (apiProvider !== undefined && !isProviderName(apiProvider)) {
logger.info(`[ContextProxy] Found invalid provider "${apiProvider}" in storage - clearing it`)
// Clear the invalid provider from both cache and storage
this.stateCache.apiProvider = undefined
await this.originalContext.globalState.update("apiProvider", undefined)
}
} catch (error) {
logger.error(
`Error during invalid API provider migration: ${error instanceof Error ? error.message : String(error)}`,
)
}
}

/**
* Migrates old nested openRouterImageGenerationSettings to the new flattened structure
*/
Expand Down Expand Up @@ -266,15 +291,38 @@ export class ContextProxy {
public getProviderSettings(): ProviderSettings {
const values = this.getValues()

// Sanitize invalid/removed apiProvider values before parsing
// This handles cases where a user had a provider selected that was later removed
// from the extension (e.g., "glama"). We sanitize here to avoid repeated
// schema validation errors that can cause infinite loops in telemetry.
const sanitizedValues = this.sanitizeProviderValues(values)

try {
return providerSettingsSchema.parse(values)
return providerSettingsSchema.parse(sanitizedValues)
} catch (error) {
if (error instanceof ZodError) {
TelemetryService.instance.captureSchemaValidationError({ schemaName: "ProviderSettings", error })
}

return PROVIDER_SETTINGS_KEYS.reduce((acc, key) => ({ ...acc, [key]: values[key] }), {} as ProviderSettings)
return PROVIDER_SETTINGS_KEYS.reduce(
(acc, key) => ({ ...acc, [key]: sanitizedValues[key] }),
{} as ProviderSettings,
)
}
}

/**
* Sanitizes provider values by resetting invalid/removed apiProvider values.
* This prevents schema validation errors for removed providers.
*/
private sanitizeProviderValues(values: RooCodeSettings): RooCodeSettings {
if (values.apiProvider !== undefined && !isProviderName(values.apiProvider)) {
logger.info(`[ContextProxy] Sanitizing invalid provider "${values.apiProvider}" - resetting to undefined`)
// Return a new values object without the invalid apiProvider
const { apiProvider, ...restValues } = values
return restValues as RooCodeSettings
}
return values
}

public async setProviderSettings(values: ProviderSettings) {
Expand Down
32 changes: 31 additions & 1 deletion src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
getModelId,
type ProviderName,
isProviderName,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

Expand Down Expand Up @@ -598,7 +599,10 @@ export class ProviderSettingsManager {

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

/**
* Sanitizes a provider config by resetting invalid/removed apiProvider values.
* This handles cases where a user had a provider selected that was later removed
* from the extension (e.g., "glama").
*/
private sanitizeProviderConfig(apiConfig: unknown): unknown {
if (typeof apiConfig !== "object" || apiConfig === null) {
return apiConfig
}

const config = apiConfig as Record<string, unknown>

// Check if apiProvider is set and if it's still valid
if (config.apiProvider !== undefined && !isProviderName(config.apiProvider)) {
console.log(
`[ProviderSettingsManager] Sanitizing invalid provider "${config.apiProvider}" - resetting to undefined`,
)
// Return a new config object without the invalid apiProvider
// This effectively resets the profile so the user can select a valid provider
const { apiProvider, ...restConfig } = config
return restConfig
}

return apiConfig
}

private async store(providerProfiles: ProviderProfiles) {
try {
await this.context.secrets.store(this.secretsKey, JSON.stringify(providerProfiles, null, 2))
Expand Down
75 changes: 75 additions & 0 deletions src/core/config/__tests__/ContextProxy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,4 +428,79 @@ describe("ContextProxy", () => {
expect(initializeSpy).toHaveBeenCalledTimes(1)
})
})

describe("invalid apiProvider migration", () => {
it("should clear invalid apiProvider from storage during initialization", async () => {
// Reset and create a new proxy with invalid provider in state
vi.clearAllMocks()
mockGlobalState.get.mockImplementation((key: string) => {
if (key === "apiProvider") {
return "invalid-removed-provider" // Invalid/removed provider
}
return undefined
})

const proxyWithInvalidProvider = new ContextProxy(mockContext)
await proxyWithInvalidProvider.initialize()

// Should have cleared the invalid apiProvider
expect(mockGlobalState.update).toHaveBeenCalledWith("apiProvider", undefined)
})

it("should not modify valid apiProvider during initialization", async () => {
// Reset and create a new proxy with valid provider in state
vi.clearAllMocks()
mockGlobalState.get.mockImplementation((key: string) => {
if (key === "apiProvider") {
return "anthropic" // Valid provider
}
return undefined
})

const proxyWithValidProvider = new ContextProxy(mockContext)
await proxyWithValidProvider.initialize()

// Should NOT have called update for apiProvider (it's valid)
const updateCalls = mockGlobalState.update.mock.calls
const apiProviderUpdateCalls = updateCalls.filter((call: any[]) => call[0] === "apiProvider")
expect(apiProviderUpdateCalls.length).toBe(0)
})
})

describe("getProviderSettings", () => {
it("should sanitize invalid apiProvider before parsing", async () => {
// Set an invalid provider in state
await proxy.updateGlobalState("apiProvider", "invalid-removed-provider" as any)
await proxy.updateGlobalState("apiModelId", "some-model")

const settings = proxy.getProviderSettings()

// The invalid apiProvider should be sanitized (removed)
expect(settings.apiProvider).toBeUndefined()
// Other settings should still be present
expect(settings.apiModelId).toBe("some-model")
})

it("should pass through valid apiProvider", async () => {
// Set a valid provider in state
await proxy.updateGlobalState("apiProvider", "anthropic")
await proxy.updateGlobalState("apiModelId", "claude-3-opus-20240229")

const settings = proxy.getProviderSettings()

// Valid provider should be returned
expect(settings.apiProvider).toBe("anthropic")
expect(settings.apiModelId).toBe("claude-3-opus-20240229")
})

it("should handle undefined apiProvider gracefully", async () => {
// Ensure no provider is set
await proxy.updateGlobalState("apiProvider", undefined)

const settings = proxy.getProviderSettings()

// Should not throw and should return undefined
expect(settings.apiProvider).toBeUndefined()
})
})
})
69 changes: 63 additions & 6 deletions src/core/config/__tests__/ProviderSettingsManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,55 @@ describe("ProviderSettingsManager", () => {
)
})

it("should remove invalid profiles during load", async () => {
it("should sanitize invalid/removed providers by resetting apiProvider to undefined", async () => {
// This tests the fix for the infinite loop issue when a provider is removed
const configWithRemovedProvider = {
currentApiConfigName: "valid",
apiConfigs: {
valid: {
apiProvider: "anthropic",
apiKey: "valid-key",
apiModelId: "claude-3-opus-20240229",
id: "valid-id",
},
removedProvider: {
// Provider that was removed from the extension (e.g., "invalid-removed-provider")
id: "removed-id",
apiProvider: "invalid-removed-provider",
apiKey: "some-key",
apiModelId: "some-model",
},
},
migrations: {
rateLimitSecondsMigrated: true,
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
todoListEnabledMigrated: true,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(configWithRemovedProvider))

await providerSettingsManager.initialize()

const storeCalls = mockSecrets.store.mock.calls
expect(storeCalls.length).toBeGreaterThan(0)
const finalStoredConfigJson = storeCalls[storeCalls.length - 1][1]

const storedConfig = JSON.parse(finalStoredConfigJson)
// The valid provider should be untouched
expect(storedConfig.apiConfigs.valid).toBeDefined()
expect(storedConfig.apiConfigs.valid.apiProvider).toBe("anthropic")

// The config with the removed provider should have its apiProvider reset to undefined
// but still be present (not filtered out entirely)
expect(storedConfig.apiConfigs.removedProvider).toBeDefined()
expect(storedConfig.apiConfigs.removedProvider.apiProvider).toBeUndefined()
expect(storedConfig.apiConfigs.removedProvider.id).toBe("removed-id")
})

it("should sanitize invalid providers and remove non-object profiles during load", async () => {
const invalidConfig = {
currentApiConfigName: "valid",
apiConfigs: {
Expand All @@ -715,12 +763,12 @@ describe("ProviderSettingsManager", () => {
apiModelId: "claude-3-opus-20240229",
rateLimitSeconds: 0,
},
invalid: {
// Invalid API provider.
invalidProvider: {
// Invalid API provider - should be sanitized (kept but apiProvider reset to undefined)
id: "x.ai",
apiProvider: "x.ai",
},
// Incorrect type.
// Incorrect type - should be completely removed
anotherInvalid: "not an object",
},
migrations: {
Expand All @@ -737,10 +785,19 @@ describe("ProviderSettingsManager", () => {
const finalStoredConfigJson = storeCalls[storeCalls.length - 1][1]

const storedConfig = JSON.parse(finalStoredConfigJson)
// Valid config should be untouched
expect(storedConfig.apiConfigs.valid).toBeDefined()
expect(storedConfig.apiConfigs.invalid).toBeUndefined()
expect(storedConfig.apiConfigs.valid.apiProvider).toBe("anthropic")

// Invalid provider config should be sanitized - kept but apiProvider reset to undefined
expect(storedConfig.apiConfigs.invalidProvider).toBeDefined()
expect(storedConfig.apiConfigs.invalidProvider.apiProvider).toBeUndefined()
expect(storedConfig.apiConfigs.invalidProvider.id).toBe("x.ai")

// Non-object config should be completely removed
expect(storedConfig.apiConfigs.anotherInvalid).toBeUndefined()
expect(Object.keys(storedConfig.apiConfigs)).toEqual(["valid"])

expect(Object.keys(storedConfig.apiConfigs)).toEqual(["valid", "invalidProvider"])
expect(storedConfig.currentApiConfigName).toBe("valid")
})
})
Expand Down
Loading