From bd5e32f6835d0890fc52e19ed7eebc430b033b77 Mon Sep 17 00:00:00 2001 From: 7Sageer Date: Thu, 27 Aug 2026 16:50:42 +0800 Subject: [PATCH] fix(secondary-model): stop rewriting the section when providers refresh or are removed (#3284) * fix(secondary-model): stop rewriting the section when providers refresh or are removed Provider refresh, provider deletion/rename, catalog/registry import, OAuth logout, and SDK removeProvider used to cascade into the user's [secondary_model] block: pool entries were silently pruned, and the whole section was deleted when its effective default dangled. The cascade ran from a cache-refresh path (including an unattended 6h scheduler), so upstream model-list changes could irreversibly destroy hand-written configuration without any notice. Config is user intent; the catalog is an availability snapshot. Stop rewriting the section on every provider/models writer. An entry whose model no longer resolves fails pool validation on the next session create with a message naming the offending alias, which is the same fail-fast contract hand-written typos already had. * chore(sdk): add changeset for the removed secondary-model cascade export * Delete .changeset/sdk-remove-secondary-model-cascade.md Signed-off-by: 7Sageer * Delete .changeset/secondary-model-no-silent-rewrite.md Signed-off-by: 7Sageer --------- Signed-off-by: 7Sageer --- apps/kimi-code/src/tui/commands/provider.ts | 15 ------ docs/en/configuration/config-files.md | 2 + docs/zh/configuration/config-files.md | 2 + .../src/app/kosongConfig/discoveryService.ts | 15 ------ .../kosongConfig/modelsDevImportService.ts | 20 -------- .../src/session/subagent/configSection.ts | 34 ------------- .../test/app/kosongConfig/discovery.test.ts | 11 ++-- .../app/kosongConfig/modelsDevImport.test.ts | 12 ++--- .../kap-server/src/routes/modelCatalog.ts | 30 ----------- .../test/modelCatalogProviderWrite.test.ts | 26 ++++++---- packages/node-sdk/src/index.ts | 4 -- packages/node-sdk/src/sdk-rpc-client-v2.ts | 27 ++++------ packages/node-sdk/src/v2/config-mapper.ts | 50 ++----------------- .../node-sdk/test/sdk-rpc-client-v2.test.ts | 49 +++++++++++------- 14 files changed, 78 insertions(+), 219 deletions(-) diff --git a/apps/kimi-code/src/tui/commands/provider.ts b/apps/kimi-code/src/tui/commands/provider.ts index d31c97006..6ef4108b4 100644 --- a/apps/kimi-code/src/tui/commands/provider.ts +++ b/apps/kimi-code/src/tui/commands/provider.ts @@ -6,7 +6,6 @@ import { } from '@moonshot-ai/kimi-code-oauth'; import { applyCatalogProvider, - cascadeSubagentModelPool, catalogProviderModels, CatalogFetchError, DEFAULT_CATALOG_URL, @@ -236,10 +235,6 @@ async function handleCatalogProviderAdd(host: SlashCommandHost): Promise { // entered. The model selector that follows is just a convenience to pick the // default model; ESC leaves the provider in place without a default selection. const existingConfig = await host.harness.getConfig(); - const poolSnapshot = - existingConfig.providers[providerId] !== undefined - ? existingConfig.secondaryModel - : undefined; if (existingConfig.providers[providerId] !== undefined) { await host.harness.removeProvider(providerId); } @@ -260,16 +255,6 @@ async function handleCatalogProviderAdd(host: SlashCommandHost): Promise { models: config.models, }); - // removeProvider cascaded the subagent pool against a model table where - // every `${providerId}/...` alias was absent; restore the entries that - // survived the re-add (aliases the catalog genuinely dropped stay dropped). - if (poolSnapshot !== undefined) { - const restored = cascadeSubagentModelPool(poolSnapshot, config.models ?? {}); - if (restored !== null) { - await host.harness.setConfig({ secondaryModel: restored ?? poolSnapshot }); - } - } - await host.authFlow.refreshConfigAfterLogin(); host.track('connect', { provider: providerId, method: 'catalog' }); host.showStatus(`Provider added: ${entry.name ?? providerId}`); diff --git a/docs/en/configuration/config-files.md b/docs/en/configuration/config-files.md index d4867efba..71d802dd6 100644 --- a/docs/en/configuration/config-files.md +++ b/docs/en/configuration/config-files.md @@ -220,6 +220,8 @@ Constraints between the fields: - `default_effort` is section-wide: every spawn binds it regardless of the chosen pool entry (or the forced model). For per-entry efforts, leave it unset and use model variants (see below). - `primary` is a reserved alias (see below) and cannot be a pool key. +Pool aliases reference the current `[models]` table: if a provider is later deleted or logged out, or its refreshed model list no longer contains an alias, session startup fails with a configuration error naming the broken alias — fix or remove the entry to recover. The `[secondary_model]` section itself is never rewritten automatically. + In the interactive TUI, the [`/secondary-model`](../reference/slash-commands.md) command (alias `/subagent-model`) opens a model selector: the choice is written to `default_model` (when a models table exists and the picked alias is not in it, an entry with an empty description is added), and newly spawned subagents pick up the new default immediately — no session restart needed. A configured pool — an explicit `models` table or a lone `default_model` — enables model selection: the `Agent` / `AgentSwarm` tools gain a `model` parameter, and the tool description lists the pool (the default marked `[default]`) so the main agent can choose per spawn. Pool keys can only reference configured [`[models]`](#models) entries — the `kimi-code/*` aliases below are provisioned by `/login`: diff --git a/docs/zh/configuration/config-files.md b/docs/zh/configuration/config-files.md index 4b13c72d7..597b352cd 100644 --- a/docs/zh/configuration/config-files.md +++ b/docs/zh/configuration/config-files.md @@ -220,6 +220,8 @@ default_model = "kimi-code/kimi-for-coding-highspeed" - `default_effort` 是节级设置：无论派生绑定到池中哪个条目（或 force 固定的模型）都生效。想按条目区分档位时不要设置它，改用下文的模型「变体」。 - `primary` 是保留字（含义见下文），不能作为池中 key。 +池别名引用的是 `[models]` 表的当前内容：如果之后删除供应商、登出账号，或其刷新后的模型列表不再包含某个别名，会话启动时会报出指明失效别名的配置错误，修正或移除对应条目即可恢复。系统不会自动改写 `[secondary_model]` 节。 + 在交互式 TUI 中，也可以用 [`/secondary-model`](../reference/slash-commands.md) 命令（别名 `/subagent-model`）打开模型选择器：选择后写入 `default_model`（已有 models 表而所选别名不在其中时，会一并补一条空描述条目），之后派生的 subagent 立即按新默认值绑定，无需重启会话。 配置了模型池（显式的 `models` 表或隐式的单条目池）即启用模型选择：`Agent` / `AgentSwarm` 工具会获得 `model` 参数，工具描述中列出模型池（默认模型标注 `[default]`），main agent 可按次派生选择模型。池 key 只能引用已配置的 [`[models]`](#models) 条目——下面的 `kimi-code/*` 别名由 `/login` 自动提供： diff --git a/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts b/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts index ef5815b82..a37b49532 100644 --- a/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts +++ b/packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts @@ -29,11 +29,6 @@ import { PROVIDERS_SECTION, THINKING_SECTION, } from './configSection'; -import { - SECONDARY_MODEL_SECTION, - cascadeSubagentModelPool, - type SecondaryModelConfig, -} from '#/session/subagent/configSection'; import { IProviderDiscoveryService, ModelCatalogChanged, @@ -219,16 +214,6 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService { if ('thinking' in patch) { sections[THINKING_SECTION] = restoreDefault ? exclusion.thinking : patch.thinking; } - const nextModels = sections[MODELS_SECTION] as Record | undefined; - if (nextModels !== undefined) { - const cascadedPool = cascadeSubagentModelPool( - this.config.inspect(SECONDARY_MODEL_SECTION).userValue, - nextModels, - ); - if (cascadedPool !== undefined) { - sections[SECONDARY_MODEL_SECTION] = cascadedPool ?? undefined; - } - } await this.config.replaceSections(sections); return { providers: diff --git a/packages/agent-core-v2/src/app/kosongConfig/modelsDevImportService.ts b/packages/agent-core-v2/src/app/kosongConfig/modelsDevImportService.ts index 2d864dbef..22681feb2 100644 --- a/packages/agent-core-v2/src/app/kosongConfig/modelsDevImportService.ts +++ b/packages/agent-core-v2/src/app/kosongConfig/modelsDevImportService.ts @@ -19,11 +19,6 @@ import { modelsDevProviderModels, resolveModelsDevImport } from './modelsDev'; import { DEFAULT_MODEL_SECTION, MODELS_SECTION, PROVIDERS_SECTION } from './configSection'; import { ModelsDevImportErrors } from './errors'; import { IKosongConfigService } from './kosongConfig'; -import { - SECONDARY_MODEL_SECTION, - cascadeSubagentModelPool, - type SecondaryModelConfig, -} from '#/session/subagent/configSection'; import { IModelsDevImportService, PROVIDER_ID_PATTERN, @@ -104,19 +99,6 @@ export class ModelsDevImportService implements IModelsDevImportService { return this.config; } - private async cascadePool( - config: IConfigService, - nextModels: Record, - ): Promise { - const cascaded = cascadeSubagentModelPool( - config.inspect(SECONDARY_MODEL_SECTION).userValue, - nextModels, - ); - if (cascaded !== undefined) { - await config.replace(SECONDARY_MODEL_SECTION, cascaded); - } - } - private async doImportModelsDevProvider( options: ImportModelsDevProviderOptions, ): Promise { @@ -185,7 +167,6 @@ export class ModelsDevImportService implements IModelsDevImportService { nextModels[`${targetId}/${model.id}`] = modelsDevModelToRecord(targetId, model); } await config.replace(MODELS_SECTION, nextModels); - await this.cascadePool(config, nextModels); const firstModel = models[0]; if (firstModel !== undefined) { @@ -274,7 +255,6 @@ export class ModelsDevImportService implements IModelsDevImportService { } await config.replace(PROVIDERS_SECTION, applied.providers as ProvidersSection); await config.replace(MODELS_SECTION, (applied.models ?? {}) as ModelsSection); - await this.cascadePool(config, applied.models ?? {}); const firstEntry = Object.values(entries)[0]; const firstModelKey = firstEntry === undefined ? undefined : Object.keys(firstEntry.models)[0]; diff --git a/packages/agent-core-v2/src/session/subagent/configSection.ts b/packages/agent-core-v2/src/session/subagent/configSection.ts index 59b2640d7..9e27bde6c 100644 --- a/packages/agent-core-v2/src/session/subagent/configSection.ts +++ b/packages/agent-core-v2/src/session/subagent/configSection.ts @@ -184,40 +184,6 @@ export function assertValidSubagentModelConfig( if (pool !== undefined) assertValidSubagentModelPool(pool, modelCatalog); } -export function cascadeSubagentModelPool( - section: SecondaryModelConfig | undefined, - survivingModels: Record, - renamedAliases: ReadonlyMap = new Map(), -): SecondaryModelConfig | null | undefined { - if (section === undefined) return undefined; - const remap = (alias: string): string => renamedAliases.get(alias) ?? alias; - const nextDefault = section.defaultModel === undefined ? undefined : remap(section.defaultModel); - const nextLegacyDefault = section.model === undefined ? undefined : remap(section.model); - const effectiveDefault = nextDefault ?? nextLegacyDefault; - if (effectiveDefault !== undefined && !(effectiveDefault in survivingModels)) return null; - - let changed = nextDefault !== section.defaultModel || nextLegacyDefault !== section.model; - let nextPool: Record | undefined; - if (section.models !== undefined) { - nextPool = {}; - for (const [alias, description] of Object.entries(section.models)) { - const key = remap(alias); - if (!(key in survivingModels)) { - changed = true; - continue; - } - if (key !== alias) changed = true; - nextPool[key] = description; - } - if (Object.keys(nextPool).length === 0) { - nextPool = undefined; - changed = true; - } - } - if (!changed) return undefined; - return { ...section, defaultModel: nextDefault, model: nextLegacyDefault, models: nextPool }; -} - export function resolveSubagentBinding( config: IConfigService, flags: IFlagService, diff --git a/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts b/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts index 042dfdedb..0f444a507 100644 --- a/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts +++ b/packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts @@ -423,7 +423,7 @@ describe('refreshProviderModels write behavior', () => { } }); - it('clears the subagent model pool when a refresh drops its default alias', async () => { + it('leaves the subagent model pool untouched when a refresh drops its default alias', async () => { const baseUrl = 'https://api.managed.example.test/coding/v1'; vi.stubEnv('KIMI_CODE_BASE_URL', baseUrl); const fetchMock = vi.fn( @@ -455,13 +455,16 @@ describe('refreshProviderModels write behavior', () => { expect(result.changed).toEqual([ { provider_id: 'my-kimi', provider_name: 'my-kimi', added: 1, removed: 1 }, ]); - expect(config.get('secondaryModel')).toBeUndefined(); + expect(config.get('secondaryModel')).toEqual({ + defaultModel: 'my-kimi/kimi-k2', + models: { 'my-kimi/kimi-k2': 'fast and cheap' }, + }); } finally { host.dispose(); } }); - it('filters pool entries a refresh dropped while keeping a surviving default', async () => { + it('leaves the whole pool untouched even when a refresh drops a non-default entry', async () => { const baseUrl = 'https://api.managed.example.test/coding/v1'; vi.stubEnv('KIMI_CODE_BASE_URL', baseUrl); const fetchMock = vi.fn( @@ -497,7 +500,7 @@ describe('refreshProviderModels write behavior', () => { ]); expect(config.get('secondaryModel')).toEqual({ defaultModel: 's1', - models: { s1: 'static fallback' }, + models: { s1: 'static fallback', 'my-kimi/kimi-k2': 'managed' }, }); } finally { host.dispose(); diff --git a/packages/agent-core-v2/test/app/kosongConfig/modelsDevImport.test.ts b/packages/agent-core-v2/test/app/kosongConfig/modelsDevImport.test.ts index 6ca0ecf7b..b34457686 100644 --- a/packages/agent-core-v2/test/app/kosongConfig/modelsDevImport.test.ts +++ b/packages/agent-core-v2/test/app/kosongConfig/modelsDevImport.test.ts @@ -236,7 +236,7 @@ describe('IModelsDevImportService', () => { expect(config.get('defaultModel')).toBe('k2'); }); - it('filters pool entries a catalog import drops, keeping a surviving default', async () => { + it('leaves the pool untouched when a catalog import drops an entry', async () => { setModelsDevUpstreamForTest({ fetchImpl: fetchJson(CATALOG) }); const { config, imports } = createHost({ providers: { openai: { type: 'openai', apiKey: 'sk-old' } }, @@ -254,11 +254,11 @@ describe('IModelsDevImportService', () => { expect(config.get('secondaryModel')).toEqual({ defaultModel: 'k2', - models: { k2: 'fast' }, + models: { k2: 'fast', 'openai/gpt-4o': 'smart' }, }); }); - it('clears the pool when a catalog import orphans its default', async () => { + it('leaves the pool untouched when a catalog import orphans its default', async () => { setModelsDevUpstreamForTest({ fetchImpl: fetchJson(CATALOG) }); const { config, imports } = createHost({ providers: { openai: { type: 'openai', apiKey: 'sk-old' } }, @@ -270,10 +270,10 @@ describe('IModelsDevImportService', () => { await imports.importModelsDevProvider({ catalogId: 'openai' }); - expect(config.get('secondaryModel')).toBeUndefined(); + expect(config.get('secondaryModel')).toEqual({ defaultModel: 'openai/gpt-4o' }); }); - it('cascades the pool on custom-registry imports too', async () => { + it('leaves the pool untouched on custom-registry imports too', async () => { setModelsDevUpstreamForTest({ fetchImpl: fetchJson(REGISTRY_DOC) }); const { config, imports } = createHost({ providers: { 'acme-gpt': { type: 'openai', apiKey: 'sk-old' } }, @@ -285,7 +285,7 @@ describe('IModelsDevImportService', () => { await imports.importCustomRegistry({ url: REGISTRY_URL }); - expect(config.get('secondaryModel')).toBeUndefined(); + expect(config.get('secondaryModel')).toEqual({ defaultModel: 'acme-gpt/gpt-old' }); }); it('seeds default_model from the first imported model only when none is configured', async () => { diff --git a/packages/kap-server/src/routes/modelCatalog.ts b/packages/kap-server/src/routes/modelCatalog.ts index afb471810..ca490bf21 100644 --- a/packages/kap-server/src/routes/modelCatalog.ts +++ b/packages/kap-server/src/routes/modelCatalog.ts @@ -21,11 +21,6 @@ import { MODELS_SECTION, PROVIDERS_SECTION, } from '@moonshot-ai/agent-core-v2/app/kosongConfig/configSection'; -import { - SECONDARY_MODEL_SECTION, - cascadeSubagentModelPool, - type SecondaryModelConfig, -} from '@moonshot-ai/agent-core-v2/session/subagent/configSection'; import { z } from 'zod'; import { errEnvelope, okEnvelope } from '../envelope'; @@ -444,24 +439,6 @@ export function registerModelCatalogRoutes(app: ModelCatalogRouteHost, core: Sco } } - const renamedAliases = new Map(); - if (newId !== provider_id) { - for (const oldAlias of previousAliasIds) { - const bare = models[oldAlias]?.model; - const renamed = bare === undefined ? undefined : `${newId}/${bare}`; - if (renamed !== undefined && nextModels[renamed] !== undefined) { - renamedAliases.set(oldAlias, renamed); - } - } - } - const secondaryModel = config.inspect( - SECONDARY_MODEL_SECTION, - ).userValue; - const cascadedPool = cascadeSubagentModelPool(secondaryModel, nextModels, renamedAliases); - if (cascadedPool !== undefined) { - await config.replace(SECONDARY_MODEL_SECTION, cascadedPool); - } - const saved = await core.accessor.get(IModelCatalog).getProvider(newId); reply.send(okEnvelope({ provider: saved }, req.id)); }); @@ -657,13 +634,6 @@ export function registerModelCatalogRoutes(app: ModelCatalogRouteHost, core: Sco if (Object.keys(restModels).length !== Object.keys(models).length) { await config.replace(MODELS_SECTION, restModels); } - const secondaryModel = config.inspect( - SECONDARY_MODEL_SECTION, - ).userValue; - const cascadedPool = cascadeSubagentModelPool(secondaryModel, restModels); - if (cascadedPool !== undefined) { - await config.replace(SECONDARY_MODEL_SECTION, cascadedPool); - } (reply as unknown as StatusReply).code(204).send(); }); }, diff --git a/packages/kap-server/test/modelCatalogProviderWrite.test.ts b/packages/kap-server/test/modelCatalogProviderWrite.test.ts index bfbcd85b1..c490e53c2 100644 --- a/packages/kap-server/test/modelCatalogProviderWrite.test.ts +++ b/packages/kap-server/test/modelCatalogProviderWrite.test.ts @@ -446,7 +446,7 @@ describe('server-v2 /api/v1 provider write endpoints', () => { }); }); - it('filters secondary_model pool entries whose provider was deleted', async () => { + it('leaves the secondary_model pool untouched when a provider is deleted', async () => { await boot(POOL_TOML); const { status } = await deleteJson('/api/v1/providers/openai'); expect(status).toBe(204); @@ -454,17 +454,20 @@ describe('server-v2 /api/v1 provider write endpoints', () => { const onDisk = await readConfigToml(); expect(onDisk['secondary_model']).toEqual({ default_model: 'k2', - models: { k2: 'fast' }, + models: { k2: 'fast', gpt4o: 'smart' }, }); }); - it('drops the secondary_model section when its default dangles after deletion', async () => { + it('keeps the secondary_model section even when its default dangles after deletion', async () => { await boot(POOL_DANGLING_DEFAULT_TOML); const { status } = await deleteJson('/api/v1/providers/openai'); expect(status).toBe(204); const onDisk = await readConfigToml(); - expect(onDisk['secondary_model']).toBeUndefined(); + expect(onDisk['secondary_model']).toEqual({ + default_model: 'gpt4o', + models: { k2: 'fast', gpt4o: 'smart' }, + }); }); it('round-trips a created provider: delete removes every trace from config.toml', async () => { @@ -710,7 +713,7 @@ describe('server-v2 /api/v1 provider write endpoints', () => { expect(onDisk['default_model']).toBe('gpt4o'); }); - it('repoints secondary_model pool entries on provider rename', async () => { + it('leaves secondary_model pool entries alone on provider rename', async () => { await boot(POOL_TOML); const { status } = await putJson('/api/v1/providers/openai', { type: 'openai', @@ -722,11 +725,11 @@ describe('server-v2 /api/v1 provider write endpoints', () => { const onDisk = await readConfigToml(); expect(onDisk['secondary_model']).toEqual({ default_model: 'k2', - models: { k2: 'fast', 'my-openai/gpt-4o': 'smart' }, + models: { k2: 'fast', gpt4o: 'smart' }, }); }); - it('filters secondary_model pool entries dropped by a provider edit', async () => { + it('leaves secondary_model pool entries alone on provider edit', async () => { await boot(POOL_TOML); const { status } = await putJson('/api/v1/providers/openai', REPLACE_BODY); expect(status).toBe(200); @@ -734,17 +737,20 @@ describe('server-v2 /api/v1 provider write endpoints', () => { const onDisk = await readConfigToml(); expect(onDisk['secondary_model']).toEqual({ default_model: 'k2', - models: { k2: 'fast' }, + models: { k2: 'fast', gpt4o: 'smart' }, }); }); - it('drops the secondary_model section when a provider edit orphans its default', async () => { + it('keeps the secondary_model section even when a provider edit orphans its default', async () => { await boot(POOL_DANGLING_DEFAULT_TOML); const { status } = await putJson('/api/v1/providers/openai', REPLACE_BODY); expect(status).toBe(200); const onDisk = await readConfigToml(); - expect(onDisk['secondary_model']).toBeUndefined(); + expect(onDisk['secondary_model']).toEqual({ + default_model: 'gpt4o', + models: { k2: 'fast', gpt4o: 'smart' }, + }); }); it('rejects a rename to an existing provider id with 40921', async () => { diff --git a/packages/node-sdk/src/index.ts b/packages/node-sdk/src/index.ts index cc1823c6c..9db4f82c9 100644 --- a/packages/node-sdk/src/index.ts +++ b/packages/node-sdk/src/index.ts @@ -82,10 +82,6 @@ export { SECONDARY_DERIVED_MODEL_ALIAS } from '@moonshot-ai/agent-core'; // caller's own model, so hosts must not offer a user alias named `primary` // as the subagent default model. export { PRIMARY_SUBAGENT_MODEL_CHOICE } from '@moonshot-ai/agent-core-v2/session/subagent/configSection'; -// Pool cascade for writes that rebuild the `[models]` table: hosts staging a -// provider overwrite (remove-then-re-add) use it to restore the still-valid -// pool entries against the final alias set. -export { cascadeSubagentModelPool } from '@moonshot-ai/agent-core-v2/session/subagent/configSection'; // Process-wide HTTP proxy bootstrap — installed once at CLI startup so all // outbound fetch honors HTTP_PROXY / HTTPS_PROXY / NO_PROXY. diff --git a/packages/node-sdk/src/sdk-rpc-client-v2.ts b/packages/node-sdk/src/sdk-rpc-client-v2.ts index 276691b62..54f9d015b 100644 --- a/packages/node-sdk/src/sdk-rpc-client-v2.ts +++ b/packages/node-sdk/src/sdk-rpc-client-v2.ts @@ -691,30 +691,30 @@ export class SDKRpcClientV2 extends SDKRpcClientBase { /** * v1's removal cascades: the provider entry, every model pointing at it, - * the default pointers when they dangle, and the `[secondary_model]` - * subagent pool entries (the section itself when its default dangles). - * The engine's own `kosong.removeProvider` only clears the - * default-provider pointer, so the full v1 cascade is computed from the - * user-layer values (see `planProviderRemoval`) and persisted as ONE - * atomic multi-section replace — the same single-write shape as v1's - * `removeKimiProvider`, so a process exit can never leave the file in a - * halfway-cascaded state. + * and the default pointers when they dangle. The engine's own + * `kosong.removeProvider` only clears the default-provider pointer, so the + * full v1 cascade is computed from the user-layer values (see + * `planProviderRemoval`) and persisted as ONE atomic multi-section + * replace — the same single-write shape as v1's `removeKimiProvider`, so a + * process exit can never leave the file in a halfway-cascaded state. The + * `[secondary_model]` section is left alone on purpose: an entry whose + * model no longer resolves fails pool validation on the next session + * create, surfacing a named error instead of silently rewriting the + * user's configuration. */ override async removeProvider(providerId: string): Promise { await this.configReady; - const [providers, models, defaultModel, defaultProvider, secondaryModel] = await Promise.all([ + const [providers, models, defaultModel, defaultProvider] = await Promise.all([ this.klient.global.config.inspect>('providers'), this.klient.global.config.inspect>>('models'), this.klient.global.config.inspect('defaultModel'), this.klient.global.config.inspect('defaultProvider'), - this.klient.global.config.inspect>('secondaryModel'), ]); const plan = planProviderRemoval({ providers: providers.userValue, models: models.userValue, defaultModel: defaultModel.userValue, defaultProvider: defaultProvider.userValue, - secondaryModel: secondaryModel.userValue, providerId, }); const sections: Record = { @@ -727,11 +727,6 @@ export class SDKRpcClientV2 extends SDKRpcClientBase { if (plan.clearDefaultProvider) { sections['defaultProvider'] = undefined; } - if (plan.secondaryModel !== undefined) { - // `null` clears the whole section; a replacement object folds the - // filtered pool into the same atomic write. - sections['secondaryModel'] = plan.secondaryModel ?? undefined; - } await this.klient.global.config.replaceSections({ sections }); return this.getConfig(); } diff --git a/packages/node-sdk/src/v2/config-mapper.ts b/packages/node-sdk/src/v2/config-mapper.ts index 7d476a838..378add9d9 100644 --- a/packages/node-sdk/src/v2/config-mapper.ts +++ b/packages/node-sdk/src/v2/config-mapper.ts @@ -89,14 +89,6 @@ export interface ProviderRemovalPlan { readonly models: Record; readonly clearDefaultModel: boolean; readonly clearDefaultProvider: boolean; - /** - * Cascade for the `[secondary_model]` subagent pool / legacy recipe: - * `undefined` = unchanged, `null` = drop the whole section (its effective - * default dangles, so the section can no longer validate), otherwise the - * replacement section with pool entries pointing at removed models - * filtered out. - */ - readonly secondaryModel: Record | null | undefined; } /** @@ -107,18 +99,16 @@ export interface ProviderRemovalPlan { * cascade through the config facade. Inputs are the USER-layer values * (`inspect().userValue`), matching v1's disk-config write base. * - * The `[secondary_model]` section cascades too: pool entries that name a - * removed model alias are filtered out, and when the effective default - * (`defaultModel`, or the legacy recipe's `model` fallback) dangles the - * whole section is dropped — a surviving `[secondary_model.models]` table - * without its default would fail pool validation on every session create. + * The `[secondary_model]` section is deliberately left untouched: it is the + * user's own configuration, and an entry whose model no longer resolves + * fails pool validation on the next session create with a message naming + * the offending alias — a loud error beats a silent rewrite. */ export function planProviderRemoval(input: { readonly providers: Record | undefined; readonly models: Record> | undefined; readonly defaultModel: string | undefined; readonly defaultProvider: string | undefined; - readonly secondaryModel?: Record; readonly providerId: string; }): ProviderRemovalPlan { const providers = { ...input.providers }; @@ -139,36 +129,9 @@ export function planProviderRemoval(input: { models, clearDefaultModel: removedDefault, clearDefaultProvider: input.defaultProvider === input.providerId, - secondaryModel: planSecondaryModelCascade(input.secondaryModel, models), }; } -/** - * Cascade the provider removal into the `[secondary_model]` section against - * the surviving model-alias table. See `ProviderRemovalPlan.secondaryModel` - * for the tri-state result. - */ -function planSecondaryModelCascade( - secondaryModel: Record | undefined, - survivingModels: Record, -): Record | null | undefined { - if (secondaryModel === undefined) return undefined; - - const defaultAlias = secondaryModel['defaultModel'] ?? secondaryModel['model']; - if (typeof defaultAlias === 'string' && !(defaultAlias in survivingModels)) { - return null; - } - - const pool = secondaryModel['models']; - if (pool === undefined || typeof pool !== 'object' || pool === null) { - return undefined; - } - const entries = Object.entries(pool as Record); - const surviving = entries.filter(([alias]) => alias in survivingModels); - if (surviving.length === entries.length) return undefined; - return { ...secondaryModel, models: Object.fromEntries(surviving) }; -} - /** * Apply the v1 remove-provider cascade to a whole `KimiConfig` in memory (no * persistence): drop the provider entry, every model pointing at it, and the @@ -183,7 +146,6 @@ export function removeProviderFromConfig(config: KimiConfig, providerId: string) models: config.models as Record> | undefined, defaultModel: config.defaultModel, defaultProvider: config.defaultProvider, - secondaryModel: config.secondaryModel as Record | undefined, providerId, }); return { @@ -192,9 +154,5 @@ export function removeProviderFromConfig(config: KimiConfig, providerId: string) models: plan.models as KimiConfig['models'], defaultModel: plan.clearDefaultModel ? undefined : config.defaultModel, defaultProvider: plan.clearDefaultProvider ? undefined : config.defaultProvider, - secondaryModel: - plan.secondaryModel === null - ? undefined - : ((plan.secondaryModel ?? config.secondaryModel) as KimiConfig['secondaryModel']), }; } diff --git a/packages/node-sdk/test/sdk-rpc-client-v2.test.ts b/packages/node-sdk/test/sdk-rpc-client-v2.test.ts index 110cd2522..4420b6e8d 100644 --- a/packages/node-sdk/test/sdk-rpc-client-v2.test.ts +++ b/packages/node-sdk/test/sdk-rpc-client-v2.test.ts @@ -886,7 +886,7 @@ key = "${titleOAuthRef.key}" } }); - it('cascades removeProvider into the secondary_model pool', async () => { + it('leaves the secondary_model pool untouched on removeProvider', async () => { const { harness } = await makeHarness(); try { await harness.setConfig({ @@ -904,24 +904,30 @@ key = "${titleOAuthRef.key}" }, }); - // Pool entries naming a removed model alias are filtered out; the - // surviving default keeps the section valid. - const filtered = await harness.removeProvider('b'); - expect(filtered.secondaryModel).toEqual({ + // Pool entries naming a removed model alias are kept as written; an + // unresolvable entry fails pool validation on the next session create. + const kept = await harness.removeProvider('b'); + expect(kept.secondaryModel).toEqual({ defaultModel: 'a/m1', - models: { 'a/m1': 'fast' }, + models: { 'a/m1': 'fast', 'b/m1': 'smart' }, }); - // When the pool's default dangles the whole section is dropped — a - // leftover models table without its default would fail pool validation - // on every session create. + // Even a dangling default leaves the whole section in place on disk. + // (`setConfig` merges per domain, so the pool table is still the one + // written above; the default now points at the provider being removed.) await harness.setConfig({ - secondaryModel: { defaultModel: 'a/m1', models: { 'a/m1': 'fast' } }, + secondaryModel: { defaultModel: 'a/m1' }, }); const cleared = await harness.removeProvider('a'); - expect(cleared.secondaryModel).toBeUndefined(); + expect(cleared.secondaryModel).toEqual({ + defaultModel: 'a/m1', + models: { 'a/m1': 'fast', 'b/m1': 'smart' }, + }); const reread = await harness.getConfig({ reload: true }); - expect(reread.secondaryModel).toBeUndefined(); + expect(reread.secondaryModel).toEqual({ + defaultModel: 'a/m1', + models: { 'a/m1': 'fast', 'b/m1': 'smart' }, + }); } finally { await harness.close(); } @@ -1360,7 +1366,7 @@ describe('removeProviderFromConfig', () => { expect(next.defaultProvider).toBe('a'); }); - it('filters secondary_model pool entries whose model alias was removed', () => { + it('leaves secondary_model pool entries alone when their model alias was removed', () => { const config = { providers: { a: { type: 'openai' }, b: { type: 'openai' } }, models: { @@ -1377,11 +1383,11 @@ describe('removeProviderFromConfig', () => { expect(next.secondaryModel).toEqual({ defaultModel: 'a/m1', - models: { 'a/m1': 'fast' }, + models: { 'a/m1': 'fast', 'b/m1': 'smart' }, }); }); - it('drops the secondary_model section when its default model dangles', () => { + it('keeps the secondary_model section even when its default model dangles', () => { const config = { providers: { a: { type: 'openai' }, b: { type: 'openai' } }, models: { @@ -1394,15 +1400,20 @@ describe('removeProviderFromConfig', () => { }, } as unknown as KimiConfig; - expect(removeProviderFromConfig(config, 'b').secondaryModel).toBeUndefined(); + expect(removeProviderFromConfig(config, 'b').secondaryModel).toEqual({ + defaultModel: 'b/m1', + models: { 'a/m1': 'fast', 'b/m1': 'smart' }, + }); - // The legacy recipe's `model` key acts as the default fallback and - // cascades the same way. + // The legacy recipe's `model` key is left alone the same way. const legacy = { ...config, secondaryModel: { model: 'b/m1', default_effort: 'low' }, } as unknown as KimiConfig; - expect(removeProviderFromConfig(legacy, 'b').secondaryModel).toBeUndefined(); + expect(removeProviderFromConfig(legacy, 'b').secondaryModel).toEqual({ + model: 'b/m1', + default_effort: 'low', + }); }); it('leaves the secondary_model section untouched when nothing dangles', () => {