diff --git a/packages/core/src/evaluation/orchestrator.ts b/packages/core/src/evaluation/orchestrator.ts index f5e2faa5..c04293d8 100644 --- a/packages/core/src/evaluation/orchestrator.ts +++ b/packages/core/src/evaluation/orchestrator.ts @@ -17,7 +17,11 @@ import { import { readJsonFile } from './file-utils.js'; import { createBuiltinProviderRegistry, createProvider } from './providers/index.js'; import { discoverProviders } from './providers/provider-discovery.js'; -import { type ResolvedTarget, resolveTargetDefinition } from './providers/targets.js'; +import { + type ResolvedTarget, + resolveDelegatedTargetDefinition, + resolveTargetDefinition, +} from './providers/targets.js'; import type { EnvLookup, Message, @@ -356,22 +360,10 @@ export async function runEvaluation( if (resolvedTargetsByName.has(name)) { return resolvedTargetsByName.get(name); } - // Follow use_target chain to find the concrete definition - let definition = targetDefinitions.get(name); + const definition = resolveDelegatedTargetDefinition(name, targetDefinitions, envLookup); if (!definition) { return undefined; } - for (let depth = 0; depth < 5; depth++) { - const useTarget = definition.use_target; - if (typeof useTarget !== 'string' || useTarget.trim().length === 0) break; - // Resolve ${{ ENV_VAR }} syntax - const envMatch = useTarget.trim().match(/^\$\{\{\s*([A-Z0-9_]+)\s*\}\}$/i); - const resolvedName = envMatch ? (envLookup[envMatch[1]] ?? '') : useTarget.trim(); - if (resolvedName.length === 0) break; - const next = targetDefinitions.get(resolvedName); - if (!next) break; - definition = next; - } const resolved = resolveTargetDefinition(definition, envLookup, evalFilePath); resolvedTargetsByName.set(name, resolved); return resolved; diff --git a/packages/core/src/evaluation/providers/index.ts b/packages/core/src/evaluation/providers/index.ts index 3f671d6b..1c7cee7d 100644 --- a/packages/core/src/evaluation/providers/index.ts +++ b/packages/core/src/evaluation/providers/index.ts @@ -19,7 +19,11 @@ import { PiCliProvider } from './pi-cli.js'; import { PiCodingAgentProvider } from './pi-coding-agent.js'; import { ProviderRegistry } from './provider-registry.js'; import type { ResolvedTarget } from './targets.js'; -import { COMMON_TARGET_SETTINGS, resolveTargetDefinition } from './targets.js'; +import { + COMMON_TARGET_SETTINGS, + resolveDelegatedTargetDefinition, + resolveTargetDefinition, +} from './targets.js'; import type { EnvLookup, Provider, TargetDefinition } from './types.js'; import { VSCodeProvider } from './vscode-provider.js'; @@ -59,7 +63,7 @@ export type { VSCodeResolvedConfig, } from './targets.js'; -export { COMMON_TARGET_SETTINGS, resolveTargetDefinition }; +export { COMMON_TARGET_SETTINGS, resolveDelegatedTargetDefinition, resolveTargetDefinition }; export { readTargetDefinitions, listTargetNames } from './targets-file.js'; export { ensureVSCodeSubagents, diff --git a/packages/core/src/evaluation/providers/targets.ts b/packages/core/src/evaluation/providers/targets.ts index 6ec0217f..6e1181c7 100644 --- a/packages/core/src/evaluation/providers/targets.ts +++ b/packages/core/src/evaluation/providers/targets.ts @@ -652,6 +652,8 @@ export const COMMON_TARGET_SETTINGS = [ 'fallbackTargets', ] as const; +const USE_TARGET_ENV_PATTERN = /^\$\{\{\s*([A-Z0-9_]+)\s*\}\}$/i; + const BASE_TARGET_SCHEMA = z .object({ name: z.string().min(1, 'target name is required'), @@ -727,6 +729,68 @@ function resolveRetryConfig(target: z.infer): RetryCo }; } +export function resolveDelegatedTargetDefinition( + name: string, + definitions: ReadonlyMap, + env: EnvLookup = process.env, +): TargetDefinition | undefined { + let definition = definitions.get(name); + if (!definition) { + return undefined; + } + + const visited = [definition.name]; + + for (let depth = 0; depth < 10; depth++) { + const rawUseTarget = + typeof definition.use_target === 'string' ? definition.use_target.trim() : undefined; + if (!rawUseTarget) { + return definition; + } + + const envMatch = rawUseTarget.match(USE_TARGET_ENV_PATTERN); + const envVarName = envMatch?.[1]; + const resolvedName = envVarName ? (env[envVarName]?.trim() ?? '') : rawUseTarget; + + if (resolvedName.length === 0) { + if (envVarName) { + throw new Error( + `Target "${definition.name}" uses use_target: \${{ ${envVarName} }}, but ${envVarName} is not set. Set ${envVarName} to the name of a concrete target (for example, "azure") before running the eval.`, + ); + } + + throw new Error( + `Target "${definition.name}" has an empty use_target value. Point it at a concrete target name before running the eval.`, + ); + } + + const next = definitions.get(resolvedName); + if (!next) { + if (envVarName) { + throw new Error( + `Target "${definition.name}" uses use_target: \${{ ${envVarName} }}, which resolved to "${resolvedName}", but no target named "${resolvedName}" exists.`, + ); + } + + throw new Error( + `Target "${definition.name}" uses use_target: "${resolvedName}", but no target named "${resolvedName}" exists.`, + ); + } + + if (visited.includes(next.name)) { + const chain = [...visited, next.name].join(' -> '); + throw new Error(`Circular use_target reference detected: ${chain}`); + } + + definition = next; + visited.push(definition.name); + } + + throw new Error( + `Target "${name}" exceeded the maximum use_target resolution depth (10). Check for a delegation loop or overly deep alias chain.`, + ); +} + export function resolveTargetDefinition( definition: TargetDefinition, env: EnvLookup = process.env, diff --git a/packages/core/test/evaluation/providers/targets.test.ts b/packages/core/test/evaluation/providers/targets.test.ts index 7b2173ae..9dc667ea 100644 --- a/packages/core/test/evaluation/providers/targets.test.ts +++ b/packages/core/test/evaluation/providers/targets.test.ts @@ -60,9 +60,49 @@ mock.module('@ai-sdk/google', () => ({ })); const providerModule = await import('../../../src/evaluation/providers/index.js'); -const { resolveTargetDefinition, createProvider } = providerModule; +const { resolveDelegatedTargetDefinition, resolveTargetDefinition, createProvider } = + providerModule; const { extractLastAssistantContent } = await import('../../../src/evaluation/providers/types.js'); +describe('resolveDelegatedTargetDefinition', () => { + it('throws a helpful error when an env-backed use_target variable is missing', () => { + const definitions = new Map([ + ['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }], + ['azure', { name: 'azure', provider: 'azure' }], + ]); + + expect(() => resolveDelegatedTargetDefinition('grader', definitions, {})).toThrow( + /GRADER_TARGET is not set/i, + ); + }); + + it('throws a helpful error when an env-backed use_target resolves to a missing target', () => { + const definitions = new Map([ + ['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }], + ]); + + expect(() => + resolveDelegatedTargetDefinition('grader', definitions, { + GRADER_TARGET: 'azure', + }), + ).toThrow(/resolved to "azure".*no target named "azure" exists/i); + }); + + it('resolves a delegated target chain to a concrete definition', () => { + const definitions = new Map([ + ['grader', { name: 'grader', use_target: '${{ GRADER_TARGET }}' }], + ['llm', { name: 'llm', use_target: 'azure' }], + ['azure', { name: 'azure', provider: 'azure' }], + ]); + + const resolved = resolveDelegatedTargetDefinition('grader', definitions, { + GRADER_TARGET: 'llm', + }); + + expect(resolved).toEqual({ name: 'azure', provider: 'azure' }); + }); +}); + describe('resolveTargetDefinition', () => { beforeEach(() => { generateTextMock.mockClear();