Skip to content

Handle per-key failures during settings import #401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edelauna merged 1 commit into from
Jun 5, 2026
Merged

Handle per-key failures during settings import #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
217 changes: 215 additions & 2 deletions src/core/config/__tests__/importExport.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ describe("importExport", () => {

// Should show warning message with short summary (not full details)
expect(showWarningMessageSpy).toHaveBeenCalledWith(
expect.stringContaining("1 profile had issues during import."),
expect.stringContaining("1 item had issues during import."),
)
expect(showWarningMessageSpy).toHaveBeenCalledWith(
expect.stringContaining("See Developer Tools console for details."),
Expand Down Expand Up @@ -1099,7 +1099,7 @@ describe("importExport", () => {

// Should show warning message with plural summary for multiple warnings
expect(showWarningMessageSpy).toHaveBeenCalledWith(
expect.stringContaining("2 profiles had issues during import."),
expect.stringContaining("2 items had issues during import."),
)
// Should log full details to console
expect(consoleWarnSpy).toHaveBeenCalledWith(
Expand All @@ -1113,6 +1113,219 @@ describe("importExport", () => {
showWarningMessageSpy.mockRestore()
consoleWarnSpy.mockRestore()
})

it("should normalize imageGenerationProvider roo while preserving other global settings", async () => {
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])

const mockFileContent = JSON.stringify({
providerProfiles: {
currentApiConfigName: "valid-profile",
apiConfigs: {
"valid-profile": {
apiProvider: "openai" as ProviderName,
apiKey: "test-key",
id: "valid-id",
},
},
},
globalSettings: {
imageGenerationProvider: "roo",
openRouterImageGenerationSelectedModel: "openrouter/model-1",
customInstructions: "Keep this setting",
},
})

;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
mockProviderSettingsManager.export.mockResolvedValue({
currentApiConfigName: "default",
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
})
mockProviderSettingsManager.listConfig.mockResolvedValue([
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
])

const result = await importSettings({
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
})

expect(result.success).toBe(true)
expect((result as { warnings?: string[] }).warnings).toEqual(
expect.arrayContaining([
expect.stringContaining("globalSettings.imageGenerationProvider"),
expect.stringContaining('unsupported value "roo"'),
]),
)

const importedGlobalSettings = mockContextProxy.setValues.mock.calls[0][0]
expect(importedGlobalSettings).toHaveProperty("imageGenerationProvider", undefined)
expect(importedGlobalSettings.openRouterImageGenerationSelectedModel).toBe("openrouter/model-1")
expect(importedGlobalSettings.customInstructions).toBe("Keep this setting")
})

it("should partially import valid global settings when invalid top-level keys are present", async () => {
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])

const mockFileContent = JSON.stringify({
providerProfiles: {
currentApiConfigName: "valid-profile",
apiConfigs: {
"valid-profile": {
apiProvider: "openai" as ProviderName,
apiKey: "test-key",
id: "valid-id",
},
},
},
globalSettings: {
customInstructions: "Keep this setting",
autoApprovalEnabled: true,
requestDelaySeconds: "slow",
telemetrySetting: "maybe",
},
})

;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
mockProviderSettingsManager.export.mockResolvedValue({
currentApiConfigName: "default",
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
})
mockProviderSettingsManager.listConfig.mockResolvedValue([
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
])

const result = await importSettings({
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
})

expect(result.success).toBe(true)
expect((result as { warnings?: string[] }).warnings).toEqual(
expect.arrayContaining([
expect.stringContaining("globalSettings.requestDelaySeconds"),
expect.stringContaining("globalSettings.telemetrySetting"),
]),
)

const importedGlobalSettings = mockContextProxy.setValues.mock.calls[0][0]
expect(importedGlobalSettings).toEqual({
customInstructions: "Keep this setting",
autoApprovalEnabled: true,
})
})

it("should skip invalid customModes without aborting unrelated settings import", async () => {
;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }])

const mockFileContent = JSON.stringify({
providerProfiles: {
currentApiConfigName: "valid-profile",
apiConfigs: {
"valid-profile": {
apiProvider: "openai" as ProviderName,
apiKey: "test-key",
id: "valid-id",
},
},
},
globalSettings: {
customInstructions: "Keep this setting",
customModes: [
{
slug: "broken-mode",
name: "",
roleDefinition: "",
groups: ["invalid-group"],
},
],
},
})

;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
mockProviderSettingsManager.export.mockResolvedValue({
currentApiConfigName: "default",
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
})
mockProviderSettingsManager.listConfig.mockResolvedValue([
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
])

const result = await importSettings({
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
})

expect(result.success).toBe(true)
expect((result as { warnings?: string[] }).warnings).toEqual(
expect.arrayContaining([expect.stringContaining("globalSettings.customModes")]),
)
expect(mockCustomModesManager.updateCustomMode).not.toHaveBeenCalled()
expect(mockContextProxy.setValues).toHaveBeenCalledWith({
customInstructions: "Keep this setting",
})
})

it("should use generic warning wording when only global settings have issues", async () => {
const filePath = "/mock/path/settings.json"
const mockFileContent = JSON.stringify({
providerProfiles: {
currentApiConfigName: "valid-profile",
apiConfigs: {
"valid-profile": {
apiProvider: "openai" as ProviderName,
apiKey: "test-key",
id: "valid-id",
},
},
},
globalSettings: {
requestDelaySeconds: "slow",
},
})

;(fs.readFile as Mock).mockResolvedValue(mockFileContent)
;(fs.access as Mock).mockResolvedValue(undefined)
mockProviderSettingsManager.export.mockResolvedValue({
currentApiConfigName: "default",
apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } },
})
mockProviderSettingsManager.listConfig.mockResolvedValue([
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
])

const mockProvider = {
settingsImportedAt: 0,
postStateToWebview: vi.fn().mockResolvedValue(undefined),
}

const showWarningMessageSpy = vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined)
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})

await importSettingsWithFeedback(
{
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
provider: mockProvider,
},
filePath,
)

expect(showWarningMessageSpy).toHaveBeenCalledWith(
expect.stringContaining("1 item had issues during import."),
)
expect(showWarningMessageSpy).not.toHaveBeenCalledWith(expect.stringContaining("profile had issues"))
expect(consoleWarnSpy).toHaveBeenCalledWith(
"Settings import completed with warnings:",
expect.arrayContaining([expect.stringContaining("globalSettings.requestDelaySeconds")]),
)

showWarningMessageSpy.mockRestore()
consoleWarnSpy.mockRestore()
})
})
})

Expand Down
70 changes: 64 additions & 6 deletions src/core/config/importExport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
globalSettingsSchema,
providerSettingsWithIdSchema,
isProviderName,
type GlobalSettings,
type ProviderSettingsWithId,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
Expand Down Expand Up @@ -70,6 +71,57 @@ function sanitizeProviderConfig(configName: string, apiConfig: unknown): { confi
return { config: apiConfig }
}

const globalSettingsShape = globalSettingsSchema.shape as Record<keyof GlobalSettings, z.ZodTypeAny>

function formatZodIssues(error: ZodError): string {
return error.issues.map((issue) => `[${issue.path.join(".") || "value"}]: ${issue.message}`).join(", ")
}

function sanitizeGlobalSettings(rawGlobalSettings: unknown): {
sanitizedGlobalSettings: GlobalSettings
warnings: string[]
} {
const warnings: string[] = []
const sanitizedGlobalSettings: Record<string, unknown> = {}

if (typeof rawGlobalSettings === "undefined") {
return { sanitizedGlobalSettings: sanitizedGlobalSettings as GlobalSettings, warnings }
}

if (typeof rawGlobalSettings !== "object" || rawGlobalSettings === null || Array.isArray(rawGlobalSettings)) {
warnings.push(
`Setting "globalSettings" was skipped: Expected object, received ${Array.isArray(rawGlobalSettings) ? "array" : typeof rawGlobalSettings}.`,
)
return { sanitizedGlobalSettings: sanitizedGlobalSettings as GlobalSettings, warnings }
}

for (const [key, rawValue] of Object.entries(rawGlobalSettings)) {
const path = `globalSettings.${key}`
const schema = globalSettingsShape[key as keyof GlobalSettings]

if (!schema) {
warnings.push(`Setting "${path}" was skipped: Unknown setting.`)
continue
}

let valueToValidate = rawValue

if (key === "imageGenerationProvider" && rawValue === "roo") {
warnings.push(`Setting "${path}" used unsupported value "roo" and was cleared during import.`)
valueToValidate = undefined
}

const result = schema.safeParse(valueToValidate)
if (result.success) {
sanitizedGlobalSettings[key] = result.data
} else {
warnings.push(`Setting "${path}" was skipped: ${formatZodIssues(result.error)}`)
}
}

return { sanitizedGlobalSettings: sanitizedGlobalSettings as GlobalSettings, warnings }
}

/**
* Imports configuration from a specific file path
* Shares base functionality for import settings for both the manual
Expand All @@ -91,14 +143,15 @@ export async function importSettingsFromPath(

const lenientSchema = z.object({
providerProfiles: lenientProviderProfilesSchema,
globalSettings: globalSettingsSchema.optional(),
globalSettings: z.unknown().optional(),
})

try {
const previousProviderProfiles = await providerSettingsManager.export()

const rawData = JSON.parse(await fs.readFile(filePath, "utf-8"))
const { providerProfiles: rawProviderProfiles, globalSettings = {} } = lenientSchema.parse(rawData)
const { providerProfiles: rawProviderProfiles, globalSettings: rawGlobalSettings } =
lenientSchema.parse(rawData)

// Track warnings for profiles that had issues
const warnings: string[] = []
Expand Down Expand Up @@ -161,15 +214,20 @@ export async function importSettingsFromPath(
},
}

const { sanitizedGlobalSettings, warnings: globalSettingsWarnings } = sanitizeGlobalSettings(rawGlobalSettings)
warnings.push(...globalSettingsWarnings)

await Promise.all(
(globalSettings.customModes ?? []).map((mode) => customModesManager.updateCustomMode(mode.slug, mode)),
(sanitizedGlobalSettings.customModes ?? []).map((mode) =>
customModesManager.updateCustomMode(mode.slug, mode),
),
)

// OpenAI Compatible settings are now correctly stored in codebaseIndexConfig
// They will be imported automatically with the config - no special handling needed

await providerSettingsManager.import(providerProfiles)
await contextProxy.setValues(globalSettings)
await contextProxy.setValues(sanitizedGlobalSettings)

// Set the current provider.
const currentProviderName = providerProfiles.currentApiConfigName
Expand All @@ -187,7 +245,7 @@ export async function importSettingsFromPath(

return {
providerProfiles,
globalSettings,
globalSettings: sanitizedGlobalSettings,
success: true,
warnings: warnings.length > 0 ? warnings : undefined,
}
Expand Down Expand Up @@ -338,7 +396,7 @@ export const importSettingsWithFeedback = async (
// Show a short summary in the toast notification
const count = warnings.length
const summary =
count === 1 ? `1 profile had issues during import.` : `${count} profiles had issues during import.`
count === 1 ? `1 item had issues during import.` : `${count} items had issues during import.`
await vscode.window.showWarningMessage(
`${t("common:info.settings_imported")} ${summary} See Developer Tools console for details.`,
)
Expand Down
Loading
Loading