From 86efbdbcea425667ab5fb0c1a8b74b2ed53d22cb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 18:42:40 +0000 Subject: [PATCH] Fix custom model objects with multiple keys being silently skipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveModelCollections checked for a single-key provider object before the custom-model (`id`) branch, so a custom model like `{ id, url, modelName }` failed the single-key check and was skipped with a confusing "Must have exactly one key" warning — the `.id` branch was unreachable dead code. Restructure to a single object branch that checks `id` first (custom model definition, passed through) and falls back to provider shorthand. Also clarify the skip warning to mention the `id` option so the failure is diagnosable. Fixes #22 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01QUWffHt992rYTqivjW2V8e --- src/cli/commands/run-config.test.ts | 25 +++++++++++++++++++++ src/cli/commands/run-config.ts | 35 ++++++++++++++++------------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/cli/commands/run-config.test.ts b/src/cli/commands/run-config.test.ts index f1a7e664..e74ac4ec 100644 --- a/src/cli/commands/run-config.test.ts +++ b/src/cli/commands/run-config.test.ts @@ -242,6 +242,31 @@ describe('run-config validation logic', () => { ); }); + it('should pass through a custom model object with multiple keys (id, url, modelName)', async () => { + const customModel = { id: 'my-custom-agent', url: 'http://localhost:3001/v1', modelName: 'gpt-4o' }; + const result = await resolveModelCollections([customModel], undefined, mockLogger as any); + + expect(result).toEqual([customModel]); + // It must NOT be treated as a malformed single-key provider object. + expect(mockLogger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('Invalid object entry in models array'), + ); + }); + + it('should pass custom model objects through alongside strings and collections', async () => { + const customModel = { id: 'my-custom-agent', url: 'http://localhost:3001/v1', modelName: 'gpt-4o' }; + const collectionsRepoPath = '/fake/repo'; + mockedFs.readFile.mockResolvedValue(JSON.stringify(['google:gemini-1.5-flash-latest'])); + + const result = await resolveModelCollections( + ['openai:gpt-4o-mini', customModel, 'TEST_COLLECTION'], + collectionsRepoPath, + mockLogger as any, + ); + + expect(result).toEqual(['openai:gpt-4o-mini', customModel, 'google:gemini-1.5-flash-latest']); + }); + it('should deduplicate models from multiple collections and literals', async () => { const models = ['openai:gpt-4o-mini', 'COLLECTION_A', 'COLLECTION_B']; const collectionsRepoPath = '/fake/repo'; diff --git a/src/cli/commands/run-config.ts b/src/cli/commands/run-config.ts index e533a7b6..b9e3d888 100644 --- a/src/cli/commands/run-config.ts +++ b/src/cli/commands/run-config.ts @@ -69,25 +69,28 @@ export async function resolveModelCollections(configModels: any[], collectionsRe } normalizedConfigModels.push(correctedEntry); } else if (typeof modelEntry === 'object' && modelEntry !== null && !Array.isArray(modelEntry)) { - const keys = Object.keys(modelEntry); - if (keys.length === 1) { - const provider = keys[0].trim(); - const modelNameValue = modelEntry[keys[0]]; - - if (typeof modelNameValue === 'string') { - const modelName = modelNameValue.trim(); - const corrected = `${provider}:${modelName}`; - logger.warn(`Found model entry as a key-value pair: ${JSON.stringify(modelEntry)}. Interpreting as '${corrected}'. For clarity, please use the string format "provider:model" in your blueprint.`); - normalizedConfigModels.push(corrected); + if (modelEntry.id) { + // Custom model definition (e.g. { id, url, modelName }). Pass through directly. + normalizedConfigModels.push(modelEntry); + } else { + // Provider shorthand — expect exactly one key: { provider: modelName }. + const keys = Object.keys(modelEntry); + if (keys.length === 1) { + const provider = keys[0].trim(); + const modelNameValue = modelEntry[keys[0]]; + + if (typeof modelNameValue === 'string') { + const modelName = modelNameValue.trim(); + const corrected = `${provider}:${modelName}`; + logger.warn(`Found model entry as a key-value pair: ${JSON.stringify(modelEntry)}. Interpreting as '${corrected}'. For clarity, please use the string format "provider:model" in your blueprint.`); + normalizedConfigModels.push(corrected); + } else { + logger.warn(`Invalid object entry in models array: Key '${provider}' has a non-string value. Skipping: ${JSON.stringify(modelEntry)}.`); + } } else { - logger.warn(`Invalid object entry in models array: Key '${provider}' has a non-string value. Skipping: ${JSON.stringify(modelEntry)}.`); + logger.warn(`Invalid object entry in models array: expected either an 'id' field (for a custom model definition) or exactly one key (the provider, as { "provider": "modelName" }). Found ${keys.length} keys and no 'id'. Skipping: ${JSON.stringify(modelEntry)}.`); } - } else { - logger.warn(`Invalid object entry in models array: Must have exactly one key (the provider). Found ${keys.length} keys. Skipping: ${JSON.stringify(modelEntry)}.`); } - } else if (typeof modelEntry === 'object' && modelEntry !== null && !Array.isArray(modelEntry) && modelEntry.id) { - // This is a custom model definition. Add it directly. - normalizedConfigModels.push(modelEntry); } else { logger.warn(`Invalid entry in models array found: ${JSON.stringify(modelEntry)}. It is not a string or a single key-value object. Skipping this entry.`); }