Skip to content

Preserve custom provider lineage during ID migrations #1025

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
PeterDaveHello merged 1 commit into from
Jul 23, 2026
Merged

Preserve custom provider lineage during ID migrations #1025

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
464 changes: 401 additions & 63 deletions src/config/index.mjs
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/popup/sections/ApiModes.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
applySelectedProviderToApiMode,
applyDeletedProviderSecrets,
applyPendingProviderChanges,
areProviderIdsEquivalent,
buildEditedProvider,
createProviderId,
getApiModeDisplayLabel,
Expand Down Expand Up @@ -400,7 +401,7 @@ export function ApiModes({ config, updateConfig }) {
return
}
const shouldClearProviderDerivedFields =
editingIndex !== -1 && selectedProviderId !== previousProviderId
editingIndex !== -1 && !areProviderIdsEquivalent(selectedProviderId, previousProviderId)
const isEndpointProviderManaged = editingIndex === -1
nextApiMode = applySelectedProviderToApiMode(
nextApiMode,
Expand Down
137 changes: 102 additions & 35 deletions src/popup/sections/api-modes-provider-utils.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ function normalizeProviderId(value) {
.replace(/^-+|-+$/g, '')
}

export function areProviderIdsEquivalent(firstProviderId, secondProviderId) {
const normalizedFirstProviderId = normalizeProviderId(firstProviderId)
const normalizedSecondProviderId = normalizeProviderId(secondProviderId)
if (!normalizedFirstProviderId || !normalizedSecondProviderId) {
return normalizeText(firstProviderId) === normalizeText(secondProviderId)
}
return normalizedFirstProviderId === normalizedSecondProviderId
}

function normalizeProviderEndpointUrl(value) {
return normalizeText(value).replace(/\/+$/, '')
}
Expand Down Expand Up @@ -95,6 +104,11 @@ export function createProviderId(providerName, existingProviders, reservedProvid
const usedIds = new Set([
...reservedProviderIds.map((providerId) => normalizeProviderId(providerId)),
...providers.map((provider) => normalizeProviderId(provider.id)),
...providers.flatMap((provider) =>
(Array.isArray(provider?.legacyProviderIds) ? provider.legacyProviderIds : []).map(
normalizeProviderId,
),
),
])

const baseId = normalizeProviderId(providerName) || `custom-provider-${providers.length + 1}`
Expand Down Expand Up @@ -310,6 +324,9 @@ export function applySelectedProviderToApiMode(
const currentProviderId = normalizeText(nextApiMode.providerId)
const nextProviderId = normalizeText(selectedProviderId)
nextApiMode.providerId = nextProviderId
if (!areProviderIdsEquivalent(currentProviderId, nextProviderId)) {
delete nextApiMode.legacyProviderIds
}
const shouldPreserveLegacyCustomUrl =
!isEndpointProviderManaged &&
!shouldClearProviderDerivedFields &&
Expand All @@ -332,6 +349,7 @@ export function sanitizeApiModeForSave(apiMode) {
nextApiMode.apiKey = ''
nextApiMode.customUrl = ''
delete nextApiMode.sourceProviderId
delete nextApiMode.legacyProviderIds
}
return nextApiMode
}
Expand Down Expand Up @@ -425,7 +443,7 @@ export function isProviderReferencedByApiModes(providerId, apiModes = []) {
return (Array.isArray(apiModes) ? apiModes : []).some(
(apiMode) =>
normalizeText(apiMode?.groupName) === 'customApiModelKeys' &&
normalizeText(apiMode?.providerId) === normalizedProviderId,
areProviderIdsEquivalent(apiMode?.providerId, normalizedProviderId),
)
}

Expand All @@ -444,12 +462,16 @@ export function getProviderDeleteDisabledReasonKey(
return ''
}

function getProvidersMatchingLegacySessionUrl(providers = [], session = null) {
function getProvidersMatchingLegacySessionUrl(
providers = [],
session = null,
{ includeDisabled = false } = {},
) {
const customUrl = normalizeProviderEndpointUrl(session?.apiMode?.customUrl)
if (!customUrl) return []

return (Array.isArray(providers) ? providers : []).filter((provider) => {
if (provider?.enabled === false) return false
if (!includeDisabled && provider?.enabled === false) return false

const directChatCompletionsUrl = normalizeProviderEndpointUrl(provider?.chatCompletionsUrl)
if (directChatCompletionsUrl && directChatCompletionsUrl === customUrl) return true
Expand All @@ -460,21 +482,25 @@ function getProvidersMatchingLegacySessionUrl(providers = [], session = null) {
})
}

function getProvidersMatchingSessionProviderId(providers = [], providerId = '') {
function getProvidersMatchingSessionProviderId(
providers = [],
providerId = '',
{ includeDisabled = false } = {},
) {
const normalizedProviderId = normalizeText(providerId)
if (!normalizedProviderId) return []

const exactMatches = (Array.isArray(providers) ? providers : []).filter(
(provider) =>
provider?.enabled !== false && normalizeText(provider?.id) === normalizedProviderId,
)
if (exactMatches.length > 0) return exactMatches

const migratedProviderId = normalizeProviderId(normalizedProviderId)
if (!migratedProviderId || migratedProviderId === normalizedProviderId) return []

return (Array.isArray(providers) ? providers : []).filter(
(provider) => provider?.enabled !== false && normalizeText(provider?.id) === migratedProviderId,
if (!migratedProviderId) return []
const availableProviders = (Array.isArray(providers) ? providers : []).filter(
(provider) => includeDisabled || provider?.enabled !== false,
)
return availableProviders.filter(
(provider) =>
normalizeText(provider?.id) === normalizedProviderId ||
(Array.isArray(provider?.legacyProviderIds) &&
provider.legacyProviderIds.map(normalizeProviderId).includes(migratedProviderId)) ||
normalizeProviderId(provider?.id) === migratedProviderId,
)
}

Expand Down Expand Up @@ -527,25 +553,32 @@ export function getReferencedCustomProviderIdsFromSessions(
const referencedProviderIds = new Set()
for (const session of Array.isArray(sessions) ? sessions : []) {
if (normalizeText(session?.apiMode?.groupName) !== 'customApiModelKeys') continue
let ambiguousProviderIdMatches = []
const providerId = normalizeText(session?.apiMode?.providerId)
if (providerId && providerId !== 'legacy-custom-default') {
const matchedProviders = getProvidersMatchingSessionProviderId(providers, providerId)
if (matchedProviders.length > 0) {
for (const provider of matchedProviders) {
const matchedProviderId = normalizeText(provider?.id)
if (matchedProviderId && matchedProviderId !== 'legacy-custom-default') {
referencedProviderIds.add(matchedProviderId)
}
const matchedProviders = getProvidersMatchingSessionProviderId(providers, providerId, {
includeDisabled: true,
})
if (matchedProviders.length === 1) {
const matchedProviderId = normalizeText(matchedProviders[0]?.id)
if (matchedProviderId && matchedProviderId !== 'legacy-custom-default') {
referencedProviderIds.add(matchedProviderId)
continue
}
continue
} else if (matchedProviders.length > 1) {
ambiguousProviderIdMatches = matchedProviders
}
if (!(Array.isArray(providers) ? providers : []).length) {
referencedProviderIds.add(providerId)
continue
}
}

const matchedByCustomUrl = getProvidersMatchingLegacySessionUrl(providers, session)
const recoveryProviders =
ambiguousProviderIdMatches.length > 0 ? ambiguousProviderIdMatches : providers
const matchedByCustomUrl = getProvidersMatchingLegacySessionUrl(recoveryProviders, session, {
includeDisabled: ambiguousProviderIdMatches.length > 0,
})
if (matchedByCustomUrl.length > 0) {
for (const provider of matchedByCustomUrl) {
const matchedProviderId = normalizeText(provider?.id)
Expand All @@ -556,15 +589,34 @@ export function getReferencedCustomProviderIdsFromSessions(
continue
}

for (const matchedProviderId of getProviderIdsMatchingSessionLabel(
session,
providers,
apiModes,
)) {
for (const provider of ambiguousProviderIdMatches) {
const matchedProviderId = normalizeText(provider?.id)
if (
provider?.enabled === false &&
areProviderIdsEquivalent(matchedProviderId, providerId) &&
matchedProviderId !== 'legacy-custom-default'
) {
referencedProviderIds.add(matchedProviderId)
}
}

const matchedProviderIdsByLabel =
ambiguousProviderIdMatches.length === 0
? getProviderIdsMatchingSessionLabel(session, providers, apiModes)
: []
for (const matchedProviderId of matchedProviderIdsByLabel) {
if (matchedProviderId && matchedProviderId !== 'legacy-custom-default') {
referencedProviderIds.add(matchedProviderId)
}
}
if (matchedProviderIdsByLabel.length === 0) {
for (const provider of ambiguousProviderIdMatches) {
const matchedProviderId = normalizeText(provider?.id)
if (matchedProviderId && matchedProviderId !== 'legacy-custom-default') {
referencedProviderIds.add(matchedProviderId)
}
}
}
}
return Array.from(referencedProviderIds)
}
Expand All @@ -577,15 +629,30 @@ export function getApiModeDisplayLabel(apiMode, t, providers = []) {
return fallbackLabel
}

const providerId = normalizeProviderId(apiMode?.providerId)
const rawProviderId = apiMode?.providerId
const providerId = normalizeProviderId(rawProviderId)
const customModelName = normalizeText(apiMode?.customName)
if (!providerId || providerId === 'legacy-custom-default') {
return fallbackLabel
}

const provider = (Array.isArray(providers) ? providers : []).find(
(item) => normalizeProviderId(item?.id) === providerId,
)
const matchedProviders = getProvidersMatchingSessionProviderId(providers, rawProviderId, {
includeDisabled: true,
})
const matchedProvidersByUrl =
matchedProviders.length > 1
? getProvidersMatchingLegacySessionUrl(
matchedProviders,
{ apiMode },
{ includeDisabled: true },
)
: []
const provider =
matchedProviders.length === 1
? matchedProviders[0]
: matchedProvidersByUrl.length === 1
? matchedProvidersByUrl[0]
: null
const providerName = normalizeText(provider?.name)
if (!providerName) return fallbackLabel
if (!customModelName) return providerName
Expand All @@ -599,9 +666,9 @@ export function getConversationAiName(session, t, providers = []) {
normalizeText(apiMode?.groupName) === 'customApiModelKeys' &&
normalizeProviderId(apiMode?.providerId) &&
normalizeProviderId(apiMode?.providerId) !== 'legacy-custom-default' &&
!(Array.isArray(providers) ? providers : []).some(
(provider) => normalizeProviderId(provider?.id) === normalizeProviderId(apiMode?.providerId),
)
getProvidersMatchingSessionProviderId(providers, apiMode?.providerId, {
includeDisabled: true,
}).length === 0

if (hasMissingCustomProvider) {
providerAwareName = ''
Expand Down
76 changes: 76 additions & 0 deletions src/popup/sections/import-data-cleanup.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
canonicalizeModelKeyArray,
canonicalizeSessionModelFields,
} from '../../config/model-key-migrations.mjs'
import { LEGACY_API_KEY_FIELD_BY_PROVIDER_ID } from '../../config/openai-provider-mappings.mjs'

const conflictingKeyPairs = [
['claudeApiKey', 'anthropicApiKey'],
Expand All @@ -13,9 +14,82 @@ const conflictingKeyPairs = [
const apiModeListKeys = ['activeApiModes', 'customApiModes', 'knownApiModeDefaultIds']
const apiModeSelectionKeys = ['modelName', 'apiMode']

function normalizeProviderId(value) {
return typeof value === 'string'
? value
.trim()
.toLowerCase()
.replace(/[^a-z0-9]+/g, '-')
.replace(/^-+|-+$/g, '')
: ''
}

async function preserveExistingBuiltinSecrets(storageArea, data, normalizedData) {
if (
!Object.hasOwn(data, 'customOpenAIProviders') ||
Object.hasOwn(data, 'providerSecrets') ||
!Array.isArray(data.customOpenAIProviders) ||
typeof storageArea.get !== 'function'
) {
return
}

const importedProviderIds = new Set(
data.customOpenAIProviders.map((provider) => normalizeProviderId(provider?.id)).filter(Boolean),
)
const relevantProviderIds = [...importedProviderIds].filter((providerId) =>
Object.hasOwn(LEGACY_API_KEY_FIELD_BY_PROVIDER_ID, providerId),
)
if (relevantProviderIds.length === 0) return

const legacyKeys = relevantProviderIds.map(
(providerId) => LEGACY_API_KEY_FIELD_BY_PROVIDER_ID[providerId],
)
const stored = await storageArea.get([
'completedBuiltinProviderIdMigrations',
'customOpenAIProviders',
'providerSecrets',
...legacyKeys,
])
const completedMigrations = new Set(
Array.isArray(stored.completedBuiltinProviderIdMigrations)
? stored.completedBuiltinProviderIdMigrations.map(normalizeProviderId).filter(Boolean)
: [],
)
const existingProviderIds = new Set(
(Array.isArray(stored.customOpenAIProviders) ? stored.customOpenAIProviders : [])
.map((provider) => normalizeProviderId(provider?.id))
.filter(Boolean),
)
const existingProviderSecrets =
stored.providerSecrets && typeof stored.providerSecrets === 'object'
? stored.providerSecrets
: {}

for (const providerId of relevantProviderIds) {
const legacyKey = LEGACY_API_KEY_FIELD_BY_PROVIDER_ID[providerId]
if (
completedMigrations.has(providerId) &&
!existingProviderIds.has(providerId) &&
Object.hasOwn(existingProviderSecrets, providerId)
) {
normalizedData[legacyKey] = existingProviderSecrets[providerId]
}
Comment on lines +45 to +77

@qodo-code-review qodo-code-review Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Import overwrites builtin secret 🐞 Bug ≡ Correctness

preserveExistingBuiltinSecrets() only restores stored builtin secrets from
providerSecrets[providerId] and ignores the legacy-key values it already fetches, so a provider-only
import can overwrite a valid legacy-only builtin key with an empty/stale imported legacy field. This
can silently drop API credentials for newly reserved builtin providers after import.
Agent Prompt 
### Issue description
`preserveExistingBuiltinSecrets()` fetches legacy builtin key fields (e.g., `xaiApiKey`) from storage but never uses their stored values. If the existing correct secret is only present in the legacy key field (or `providerSecrets[providerId]` is missing/stale) and the import payload contains that legacy key as empty/stale, `importDataIntoStorage()` will overwrite the correct value.

### Issue Context
- This helper runs specifically when importing `customOpenAIProviders` **without** importing `providerSecrets`, which is exactly when protecting existing secrets matters most.
- The code already requests `...legacyKeys` from storage, so the intent appears to be to preserve those values, but the implementation only consults `stored.providerSecrets`.

### Fix Focus Areas
- src/popup/sections/import-data-cleanup.mjs[27-79]

### Suggested fix
- When iterating `relevantProviderIds`, compute the existing builtin secret with a precedence such as:
  1) `stored.providerSecrets[providerId]` if present and non-empty, else
  2) `stored[legacyKey]` (trimmed) if present and non-empty.
- If that existing secret is non-empty and the import would otherwise set `normalizedData[legacyKey]` to empty/stale (at minimum: empty/missing), write `normalizedData[legacyKey] = existingSecret`.
- Add/extend a unit test for the case where:
  - `completedBuiltinProviderIdMigrations` contains the providerId,
  - `stored.providerSecrets` is missing the providerId entry,
  - `stored[legacyKey]` has the real secret,
  - import payload would set `legacyKey` to `''` (or includes it as empty),
  - and verify the stored secret is preserved after import + subsequent `getUserConfig()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
}

export function prepareImportData(data) {
const normalizedData = { ...data }
const keysToRemove = []
const importsCompleteProviderState =
Object.hasOwn(data, 'customOpenAIProviders') && Object.hasOwn(data, 'providerSecrets')

if (
importsCompleteProviderState &&
!Object.hasOwn(data, 'completedBuiltinProviderIdMigrations')
) {
normalizedData.completedBuiltinProviderIdMigrations = []
}

if (apiModeListKeys.some((key) => Object.hasOwn(data, key))) {
for (const key of apiModeListKeys) {
Expand Down Expand Up @@ -69,6 +143,8 @@ export function prepareImportData(data) {
export async function importDataIntoStorage(storageArea, data) {
const { normalizedData, keysToRemove } = prepareImportData(data)

await preserveExistingBuiltinSecrets(storageArea, data, normalizedData)

await storageArea.set(normalizedData)

if (keysToRemove.length > 0) {
Expand Down
Loading