diff --git a/.changeset/fix-custom-registry-provider-import.md b/.changeset/fix-custom-registry-provider-import.md new file mode 100644 index 0000000000..2e85000cb0 --- /dev/null +++ b/.changeset/fix-custom-registry-provider-import.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/kimi-code-oauth": patch +"@moonshot-ai/kimi-code": patch +--- + +Fix custom registry provider handling during re-import. Prevent loss of multi-provider entries and remove stale providers along with their model aliases and default model references. diff --git a/apps/kimi-code/src/tui/commands/provider.ts b/apps/kimi-code/src/tui/commands/provider.ts index 7690a805a5..15b5b705f8 100644 --- a/apps/kimi-code/src/tui/commands/provider.ts +++ b/apps/kimi-code/src/tui/commands/provider.ts @@ -1,7 +1,8 @@ import { - applyCustomRegistryProvider, + applyCustomRegistryEntries, fetchCustomRegistry, type CustomRegistrySource, + type ManagedKimiConfigShape, } from '@moonshot-ai/kimi-code-oauth'; import { applyCatalogProvider, @@ -268,7 +269,7 @@ async function handleCustomRegistryAddViaDialog(host: SlashCommandHost): Promise apiKey: value.apiKey, }; - let entries: Record }>; + let entries: Awaited>; try { entries = await fetchCustomRegistry(source); } catch (err) { @@ -276,20 +277,14 @@ async function handleCustomRegistryAddViaDialog(host: SlashCommandHost): Promise return false; } - const addedProviderIds: string[] = []; + const addedProviderIds = Object.values(entries).map((entry) => entry.id); try { - let config = await host.harness.getConfig(); - for (const entry of Object.values(entries)) { - if (config.providers[entry.id] !== undefined) { - config = await host.harness.removeProvider(entry.id); - } - applyCustomRegistryProvider( - config as unknown as Parameters[0], - entry as Parameters[1], - source, - ); - addedProviderIds.push(entry.id); - } + const config = await host.harness.getConfig(); + applyCustomRegistryEntries( + config as unknown as ManagedKimiConfigShape, + entries, + source, + ); await host.harness.setConfig({ providers: config.providers, models: config.models, diff --git a/packages/oauth/src/custom-registry.ts b/packages/oauth/src/custom-registry.ts index 565fea4f9e..cad5fd9f2e 100644 --- a/packages/oauth/src/custom-registry.ts +++ b/packages/oauth/src/custom-registry.ts @@ -349,3 +349,50 @@ export function removeCustomRegistryProvider( config['defaultProvider'] = undefined; } } + +/** + * Applies every entry from a single api.json import in memory. Mirrors the + * "remove if present, then apply" sequence the Add Platform flow used to do + * via the `removeProvider` RPC, but stays purely in-memory so callers can + * persist the whole batch with a single write at the end. + * + * Bug fixed: previously the caller interleaved in-memory `applyCustomRegistry- + * Provider` with the disk-writing `removeProvider` RPC inside a loop. Each + * RPC re-read disk and returned a fresh config object, discarding entries that + * had already been merged in-memory from earlier iterations. Re-importing a + * multi-provider api.json silently lost N-1 of N providers. + * + * Re-import semantics: providers previously imported from the same source URL + * but no longer present in `entries` are removed (along with their aliases and + * any `defaultModel` pointing at them). Without this, deleting a provider + * upstream and re-importing the registry leaves orphaned provider records and + * model aliases behind. Matching is by `source.url` only — the apiKey commonly + * rotates between imports, but the URL is the stable identity of "the same + * registry". + */ +export function applyCustomRegistryEntries( + config: ManagedKimiConfigShape, + entries: Record, + source: CustomRegistrySource, +): void { + const surviving = new Set(Object.values(entries).map((entry) => entry.id)); + for (const [providerId, provider] of Object.entries(config.providers)) { + if (surviving.has(providerId)) continue; + if (!isRecord(provider)) continue; + const existingSource = provider['source']; + if ( + isRecord(existingSource) && + existingSource['kind'] === 'apiJson' && + existingSource['url'] === source.url + ) { + removeCustomRegistryProvider(config, providerId); + } + } + + for (const entry of Object.values(entries)) { + if (entry.id in config.providers) { + removeCustomRegistryProvider(config, entry.id); + } + applyCustomRegistryProvider(config, entry, source); + } +} diff --git a/packages/oauth/src/index.ts b/packages/oauth/src/index.ts index 45df8a78e5..4aeda80e3c 100644 --- a/packages/oauth/src/index.ts +++ b/packages/oauth/src/index.ts @@ -99,6 +99,7 @@ export type { } from './open-platform'; export { + applyCustomRegistryEntries, applyCustomRegistryProvider, capabilitiesFromCustomEntry, CustomRegistryApiError, diff --git a/packages/oauth/test/custom-registry.test.ts b/packages/oauth/test/custom-registry.test.ts index a4d6047e2d..f781ce5a46 100644 --- a/packages/oauth/test/custom-registry.test.ts +++ b/packages/oauth/test/custom-registry.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import { + applyCustomRegistryEntries, applyCustomRegistryProvider, CUSTOM_REGISTRY_DEFAULT_CAPABILITIES, CUSTOM_REGISTRY_DEFAULT_MAX_CONTEXT, @@ -382,6 +383,187 @@ describe('removeCustomRegistryProvider', () => { }); }); +describe('applyCustomRegistryEntries', () => { + // Regression: re-importing the same multi-provider api.json used to lose all + // but the last provider because the caller mixed in-memory mutations with the + // `harness.removeProvider` RPC (which read/wrote disk inside the loop and + // returned a fresh config object, discarding prior iterations' additions). + // The pure in-memory helper must keep every entry across repeated imports. + it('keeps every provider when the same multi-provider source is applied twice', () => { + const source: CustomRegistrySource = { + kind: 'apiJson', + url: 'https://registry.example.test/v1/models/api.json', + apiKey: 'sk-token', + }; + const entries: Record = { + a: { id: 'a', name: 'A', api: 'https://a.test/v1', type: 'openai', models: { 'm1': { id: 'm1' } } }, + b: { id: 'b', name: 'B', api: 'https://b.test/v1', type: 'openai', models: { 'm1': { id: 'm1' } } }, + c: { id: 'c', name: 'C', api: 'https://c.test/v1', type: 'openai', models: { 'm1': { id: 'm1' } } }, + }; + + const config: ManagedKimiConfigShape = { providers: {} }; + applyCustomRegistryEntries(config, entries, source); + applyCustomRegistryEntries(config, entries, source); + + expect(Object.keys(config.providers).sort()).toEqual(['a', 'b', 'c']); + expect(config.models?.['a/m1']).toBeDefined(); + expect(config.models?.['b/m1']).toBeDefined(); + expect(config.models?.['c/m1']).toBeDefined(); + }); + + it('refreshes provider fields, drops stale aliases, and clears defaultModel that no longer exists', () => { + const source: CustomRegistrySource = { + kind: 'apiJson', + url: 'https://registry.example.test/api.json', + apiKey: 'sk-new', + }; + const config: ManagedKimiConfigShape = { + providers: { + x: { type: 'openai', baseUrl: 'https://x-old.test/v1', apiKey: 'sk-old' }, + }, + models: { + 'x/old-model': { provider: 'x', model: 'old-model', maxContextSize: 1000 }, + 'other/keep': { provider: 'other', model: 'keep', maxContextSize: 1000 }, + }, + defaultModel: 'x/old-model', + }; + + applyCustomRegistryEntries( + config, + { + x: { + id: 'x', + name: 'X', + api: 'https://x-new.test/v1', + type: 'openai', + models: { 'new-model': { id: 'new-model' } }, + }, + }, + source, + ); + + expect(config.providers['x']).toMatchObject({ + type: 'openai', + baseUrl: 'https://x-new.test/v1', + apiKey: 'sk-new', + }); + expect(config.models?.['x/old-model']).toBeUndefined(); + expect(config.models?.['x/new-model']).toBeDefined(); + expect(config.models?.['other/keep']).toBeDefined(); + // defaultModel pointed at the now-removed alias, so it must be cleared + // (matches the old harness.removeProvider semantics that the caller relied + // on before the refactor). + expect(config.defaultModel).toBeUndefined(); + }); + + // Re-import semantics: when a provider that existed in a previous import is + // gone from the new fetch (same source URL), it must be removed along with + // its aliases and any `defaultModel` pointing at it. Without this, deleting a + // provider upstream silently leaves orphan records and a dangling default. + it('removes providers from the same source URL that disappeared on re-import', () => { + const source: CustomRegistrySource = { + kind: 'apiJson', + url: 'https://registry.example.test/api.json', + apiKey: 'sk-token', + }; + const firstEntries: Record = { + a: { id: 'a', name: 'A', api: 'https://a.test/v1', type: 'openai', models: { m1: { id: 'm1' } } }, + b: { id: 'b', name: 'B', api: 'https://b.test/v1', type: 'openai', models: { m1: { id: 'm1' } } }, + }; + + const config: ManagedKimiConfigShape = { + providers: { + // Provider from an unrelated source — must not be touched. + keepme: { + type: 'openai', + baseUrl: 'https://keepme.test/v1', + apiKey: 'sk-keepme', + source: { + kind: 'apiJson', + url: 'https://other.example.test/api.json', + apiKey: 'sk-other', + }, + }, + }, + models: { + 'keepme/m1': { provider: 'keepme', model: 'm1', maxContextSize: 1000 }, + }, + }; + + applyCustomRegistryEntries(config, firstEntries, source); + // After first import, default points at one of the now-orphaned aliases. + config.defaultModel = 'b/m1'; + + // Second import drops provider `b`. + applyCustomRegistryEntries( + config, + { + a: firstEntries['a'] as CustomRegistryProviderEntry, + }, + source, + ); + + expect(config.providers['a']).toBeDefined(); + expect(config.providers['b']).toBeUndefined(); + expect(config.models?.['a/m1']).toBeDefined(); + expect(config.models?.['b/m1']).toBeUndefined(); + expect(config.defaultModel).toBeUndefined(); + + // Unrelated provider from a different source URL is preserved. + expect(config.providers['keepme']).toBeDefined(); + expect(config.models?.['keepme/m1']).toBeDefined(); + }); + + it('does not remove providers from a different source URL even when ids overlap', () => { + const sourceA: CustomRegistrySource = { + kind: 'apiJson', + url: 'https://registry-a.example.test/api.json', + apiKey: 'sk-a', + }; + const sourceB: CustomRegistrySource = { + kind: 'apiJson', + url: 'https://registry-b.example.test/api.json', + apiKey: 'sk-b', + }; + + const config: ManagedKimiConfigShape = { providers: {} }; + applyCustomRegistryEntries( + config, + { + shared: { + id: 'shared', + name: 'Shared', + api: 'https://shared.test/v1', + type: 'openai', + models: { m1: { id: 'm1' } }, + }, + }, + sourceB, + ); + + // Importing source A with no overlapping ids must not delete the provider + // sitting under source B. + applyCustomRegistryEntries( + config, + { + onlyA: { + id: 'onlyA', + name: 'Only A', + api: 'https://onlya.test/v1', + type: 'openai', + models: { m1: { id: 'm1' } }, + }, + }, + sourceA, + ); + + expect(config.providers['shared']).toBeDefined(); + expect(config.providers['onlyA']).toBeDefined(); + expect(config.models?.['shared/m1']).toBeDefined(); + expect(config.models?.['onlyA/m1']).toBeDefined(); + }); +}); + describe('capabilitiesFromCustomEntry', () => { it('returns an empty array when no rich fields are present', () => { expect(capabilitiesFromCustomEntry({ id: 'm' })).toEqual([]);