diff --git a/.changeset/warn-malformed-models-entry.md b/.changeset/warn-malformed-models-entry.md new file mode 100644 index 00000000000..0b74fb051cd --- /dev/null +++ b/.changeset/warn-malformed-models-entry.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Warn at startup when a [models] entry in config.toml is missing the model field and cannot be used. diff --git a/packages/agent-core-v2/src/app/config/config.ts b/packages/agent-core-v2/src/app/config/config.ts index 9003c1d36ea..fc0db298859 100644 --- a/packages/agent-core-v2/src/app/config/config.ts +++ b/packages/agent-core-v2/src/app/config/config.ts @@ -24,6 +24,8 @@ export interface ConfigKeyDeprecation { readonly message?: string; } +export type ConfigCollectDiagnostics = (rawSection: unknown) => readonly ConfigDiagnostic[]; + export type EnvBindings = EnvBinding | { [K in keyof T]?: EnvBinding | EnvBindings }; export type AnyEnvBindings = EnvBinding | { readonly [key: string]: EnvBinding | AnyEnvBindings }; @@ -93,6 +95,7 @@ export interface ConfigSection { readonly fromToml?: ConfigFromToml; readonly toToml?: ConfigToToml; readonly deprecations?: readonly ConfigKeyDeprecation[]; + readonly collectDiagnostics?: ConfigCollectDiagnostics; } export interface RegisterSectionOptions { @@ -104,6 +107,7 @@ export interface RegisterSectionOptions { readonly fromToml?: ConfigFromToml; readonly toToml?: ConfigToToml; readonly deprecations?: readonly ConfigKeyDeprecation[]; + readonly collectDiagnostics?: ConfigCollectDiagnostics; } export interface ConfigEffectiveOverlay { diff --git a/packages/agent-core-v2/src/app/config/configService.ts b/packages/agent-core-v2/src/app/config/configService.ts index fdc2dc6f5aa..fadb6a96ff5 100644 --- a/packages/agent-core-v2/src/app/config/configService.ts +++ b/packages/agent-core-v2/src/app/config/configService.ts @@ -151,7 +151,8 @@ function isSameSection( existing.fromToml === options.fromToml && existing.toToml === options.toToml && deepEqual(existing.defaultValue, options.defaultValue) && - deepEqual(existing.deprecations, options.deprecations) + deepEqual(existing.deprecations, options.deprecations) && + existing.collectDiagnostics === options.collectDiagnostics ); } @@ -249,6 +250,7 @@ export class ConfigRegistry extends Disposable implements IConfigRegistry { fromToml: options.fromToml, toToml: options.toToml, deprecations: options.deprecations, + collectDiagnostics: options.collectDiagnostics, }); this._onDidRegisterSection.fire({ domain }); } @@ -576,6 +578,13 @@ export class ConfigService extends Disposable implements IConfigService { for (const diagnostic of collectKeyDeprecations(nextRawSnake, this.registry.listSections())) { this.pushDiagnostic(diagnostic); } + for (const section of this.registry.listSections()) { + if (section.collectDiagnostics === undefined) continue; + const rawSection = nextRawSnake[camelToSnake(section.domain)]; + for (const diagnostic of section.collectDiagnostics(rawSection)) { + this.pushDiagnostic(diagnostic); + } + } if (source !== 'load' && JSON.stringify(nextRawSnake) === JSON.stringify(this.rawSnake)) { const scratch = { ...this.validated }; this.applySectionEnvBindings(scratch, true); diff --git a/packages/agent-core-v2/src/app/kosongConfig/configSection.ts b/packages/agent-core-v2/src/app/kosongConfig/configSection.ts index 992e9b3bc67..73b3c548589 100644 --- a/packages/agent-core-v2/src/app/kosongConfig/configSection.ts +++ b/packages/agent-core-v2/src/app/kosongConfig/configSection.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; import { + type ConfigDiagnostic, type ConfigStripEnv, envBindings, } from '#/app/config/config'; @@ -195,6 +196,54 @@ type _AssertModelsSection = AssertExact< Equal, ModelsSection> >; +const MODEL_OBJECT_FIELDS = new Set( + Object.entries(ModelRecordSchema.shape) + .filter(([, field]) => unwrapWrapperSchema(field as z.ZodTypeAny) instanceof z.ZodObject) + .map(([key]) => camelToSnake(key)), +); + +function unwrapWrapperSchema(schema: z.ZodTypeAny): z.ZodTypeAny { + let current = schema; + while ( + current instanceof z.ZodOptional || + current instanceof z.ZodNullable || + current instanceof z.ZodDefault + ) { + current = current.unwrap() as z.ZodTypeAny; + } + return current; +} + +function collectMalformedModelEntries(rawModels: unknown): ConfigDiagnostic[] { + if (!isPlainObject(rawModels)) return []; + const diagnostics: ConfigDiagnostic[] = []; + for (const [alias, entry] of Object.entries(rawModels)) { + if (!isPlainObject(entry)) continue; + if (entry['model'] !== undefined || entry['name'] !== undefined) continue; + diagnostics.push({ + domain: MODELS_SECTION, + severity: 'warning', + message: malformedModelMessage(alias, entry), + }); + } + return diagnostics; +} + +function malformedModelMessage(alias: string, entry: Record): string { + const base = `[models] entry '${alias}' is missing the 'model' field and cannot be used as a model`; + const dottedAlias = dottedAliasSuffix(alias, entry); + if (dottedAlias === undefined) return `${base}.`; + return `${base}; if the alias contains dots, quote the table name (e.g. [models."${dottedAlias}"]).`; +} + +function dottedAliasSuffix(alias: string, entry: Record): string | undefined { + for (const [key, value] of Object.entries(entry)) { + if (MODEL_OBJECT_FIELDS.has(key) || !isPlainObject(value)) continue; + return dottedAliasSuffix(`${alias}.${key}`, value) ?? `${alias}.${key}`; + } + return undefined; +} + export const modelsFromToml = (rawSnake: unknown): unknown => { if (!isPlainObject(rawSnake)) return rawSnake; const out: Record = {}; @@ -255,6 +304,7 @@ registerConfigSection(MODELS_SECTION, ModelsSectionSchema, { defaultValue: {}, fromToml: modelsFromToml, toToml: modelsToToml, + collectDiagnostics: collectMalformedModelEntries, }); export const THINKING_SECTION = 'thinking'; diff --git a/packages/agent-core-v2/test/app/config/config.test.ts b/packages/agent-core-v2/test/app/config/config.test.ts index d06a9827d36..87dce676ab7 100644 --- a/packages/agent-core-v2/test/app/config/config.test.ts +++ b/packages/agent-core-v2/test/app/config/config.test.ts @@ -1373,6 +1373,98 @@ describe('config deprecations', () => { }); }); +describe('malformed models config entries', () => { + async function createConfig(toml: string) { + const disposables = new DisposableStore(); + const ix = disposables.add(new TestInstantiationService()); + const storage = new InMemoryStorageService(); + await storage.write('', 'config.toml', new TextEncoder().encode(toml)); + ix.stub(ILogService, stubLog()); + ix.stub(IBootstrapService, stubBootstrap('/tmp/kimi-cfg', {})); + ix.stub(IFileSystemStorageService, storage); + ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); + ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); + ix.set(IConfigService, new SyncDescriptor(ConfigService)); + const config = ix.get(IConfigService); + await config.ready; + return { config, disposables, storage }; + } + + it('warns at load time when a dotted alias parses as a nested table', async () => { + const { config, disposables } = await createConfig( + '[models.kimi-k2.7-code]\nmodel = "kimi-k2.7-code"\nmax_context_size = 262144\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'kimi-k2' is missing the 'model' field and cannot be used as a model; " + + 'if the alias contains dots, quote the table name (e.g. [models."kimi-k2.7-code"]).', + }); + + disposables.dispose(); + }); + + it('stays silent for quoted dotted aliases and entries with a wire-facing name', async () => { + const { config, disposables } = await createConfig( + '[models."kimi-k2.7-code"]\nmodel = "kimi-k2.7-code"\n\n[models.renamed]\nname = "wire-name"\n', + ); + + expect(config.diagnostics()).toEqual([]); + + disposables.dispose(); + }); + + it('warns without the dotted-alias hint when the entry has no nested table', async () => { + const { config, disposables } = await createConfig( + '[models.partial]\nmax_context_size = 262144\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'partial' is missing the 'model' field and cannot be used as a model.", + }); + + disposables.dispose(); + }); + + it('does not mistake schema object fields for a dotted alias', async () => { + const { config, disposables } = await createConfig( + '[models.partial]\noverrides = { max_output_size = 8192 }\n', + ); + + expect(config.diagnostics()).toContainEqual({ + domain: 'models', + severity: 'warning', + message: + "[models] entry 'partial' is missing the 'model' field and cannot be used as a model.", + }); + + disposables.dispose(); + }); + + it('clears the warning on reload once the entry is fixed', async () => { + const { config, disposables, storage } = await createConfig( + '[models.kimi-k2.7-code]\nmodel = "kimi-k2.7-code"\n', + ); + expect(config.diagnostics()).toHaveLength(1); + + await storage.write( + '', + 'config.toml', + new TextEncoder().encode('[models."kimi-k2.7-code"]\nmodel = "kimi-k2.7-code"\n'), + ); + await config.reload(); + + expect(config.diagnostics()).toEqual([]); + + disposables.dispose(); + }); +}); + describe('task config section', () => { it('re-applies the keepAliveOnExit env binding on every get()', async () => { const env: Record = {}; @@ -2445,7 +2537,7 @@ describe('config section collection fold (D12)', () => { parse(value: unknown): RuntimeFoldDemo { const demo = value as RuntimeFoldDemo; if (typeof demo?.enabled !== 'boolean') { - throw new Error('runtimeFoldDemo.enabled must be a boolean'); + throw new TypeError('runtimeFoldDemo.enabled must be a boolean'); } return demo; }, @@ -2731,8 +2823,8 @@ describe('ConfigService replaceSections', () => { defaultModel: undefined, thinking: {}, }); - expect([...domains].sort()).toEqual( - [PROVIDERS_SECTION, MODELS_SECTION, DEFAULT_MODEL_SECTION, THINKING_SECTION].sort(), + expect([...domains].toSorted()).toEqual( + [PROVIDERS_SECTION, MODELS_SECTION, DEFAULT_MODEL_SECTION, THINKING_SECTION].toSorted(), ); disposables.dispose(); @@ -2810,7 +2902,7 @@ describe('ConfigService persistence guards', () => { async function expectPersistBlocked(promise: Promise): Promise { const error = await promise.then( () => undefined, - (e: unknown) => e, + (error: unknown) => error, ); expect(isError2(error)).toBe(true); expect((error as Error2).code).toBe(ErrorCodes.CONFIG_PERSIST_BLOCKED);