From d66ec38f82909eb291788328cd16af6c6584fbd3 Mon Sep 17 00:00:00 2001 From: joshualitt Date: Tue, 13 Jan 2026 14:31:34 -0800 Subject: [PATCH] feat(core): Align internal agent settings with configs exposed through settings.json (#16458) --- packages/core/src/agents/agentLoader.test.ts | 6 +- packages/core/src/agents/agentLoader.ts | 10 +- packages/core/src/agents/cli-help-agent.ts | 15 +- .../core/src/agents/codebase-investigator.ts | 15 +- .../src/agents/delegate-to-agent-tool.test.ts | 10 +- .../core/src/agents/local-executor.test.ts | 28 ++-- packages/core/src/agents/local-executor.ts | 14 +- .../core/src/agents/local-invocation.test.ts | 10 +- packages/core/src/agents/registry.test.ts | 29 ++-- packages/core/src/agents/registry.ts | 149 +++++++++--------- .../src/agents/subagent-tool-wrapper.test.ts | 10 +- packages/core/src/agents/types.ts | 15 +- 12 files changed, 176 insertions(+), 135 deletions(-) diff --git a/packages/core/src/agents/agentLoader.test.ts b/packages/core/src/agents/agentLoader.test.ts index 199d715fe9..088b233177 100644 --- a/packages/core/src/agents/agentLoader.test.ts +++ b/packages/core/src/agents/agentLoader.test.ts @@ -245,10 +245,12 @@ Body`); }, modelConfig: { model: 'inherit', - top_p: 0.95, + generateContentConfig: { + topP: 0.95, + }, }, runConfig: { - max_time_minutes: 5, + maxTimeMinutes: 5, }, inputConfig: { inputs: { diff --git a/packages/core/src/agents/agentLoader.ts b/packages/core/src/agents/agentLoader.ts index f8283c1933..b08979bbe4 100644 --- a/packages/core/src/agents/agentLoader.ts +++ b/packages/core/src/agents/agentLoader.ts @@ -281,12 +281,14 @@ export function markdownToAgentDefinition( }, modelConfig: { model: modelName, - temp: markdown.temperature ?? 1, - top_p: 0.95, + generateContentConfig: { + temperature: markdown.temperature ?? 1, + topP: 0.95, + }, }, runConfig: { - max_turns: markdown.max_turns, - max_time_minutes: markdown.timeout_mins || 5, + maxTurns: markdown.max_turns, + maxTimeMinutes: markdown.timeout_mins || 5, }, toolConfig: markdown.tools ? { diff --git a/packages/core/src/agents/cli-help-agent.ts b/packages/core/src/agents/cli-help-agent.ts index ea6709ca86..71a020f3a8 100644 --- a/packages/core/src/agents/cli-help-agent.ts +++ b/packages/core/src/agents/cli-help-agent.ts @@ -50,14 +50,19 @@ export const CliHelpAgent = ( modelConfig: { model: GEMINI_MODEL_ALIAS_FLASH, - temp: 0.1, - top_p: 0.95, - thinkingBudget: -1, + generateContentConfig: { + temperature: 0.1, + topP: 0.95, + thinkingConfig: { + includeThoughts: true, + thinkingBudget: -1, + }, + }, }, runConfig: { - max_time_minutes: 3, - max_turns: 10, + maxTimeMinutes: 3, + maxTurns: 10, }, toolConfig: { diff --git a/packages/core/src/agents/codebase-investigator.ts b/packages/core/src/agents/codebase-investigator.ts index ddff2e8500..7e0b7fd3cf 100644 --- a/packages/core/src/agents/codebase-investigator.ts +++ b/packages/core/src/agents/codebase-investigator.ts @@ -71,14 +71,19 @@ export const CodebaseInvestigatorAgent: LocalAgentDefinition< modelConfig: { model: DEFAULT_GEMINI_MODEL, - temp: 0.1, - top_p: 0.95, - thinkingBudget: -1, + generateContentConfig: { + temperature: 0.1, + topP: 0.95, + thinkingConfig: { + includeThoughts: true, + thinkingBudget: -1, + }, + }, }, runConfig: { - max_time_minutes: 5, - max_turns: 15, + maxTimeMinutes: 5, + maxTurns: 15, }, toolConfig: { diff --git a/packages/core/src/agents/delegate-to-agent-tool.test.ts b/packages/core/src/agents/delegate-to-agent-tool.test.ts index 65dcd2b2e0..b709c01911 100644 --- a/packages/core/src/agents/delegate-to-agent-tool.test.ts +++ b/packages/core/src/agents/delegate-to-agent-tool.test.ts @@ -50,14 +50,20 @@ describe('DelegateToAgentTool', () => { name: 'test_agent', description: 'A test agent', promptConfig: {}, - modelConfig: { model: 'test-model', temp: 0, top_p: 0 }, + modelConfig: { + model: 'test-model', + generateContentConfig: { + temperature: 0, + topP: 0, + }, + }, inputConfig: { inputs: { arg1: { type: 'string', description: 'Argument 1', required: true }, arg2: { type: 'number', description: 'Argument 2', required: false }, }, }, - runConfig: { max_turns: 1, max_time_minutes: 1 }, + runConfig: { maxTurns: 1, maxTimeMinutes: 1 }, toolConfig: { tools: [] }, }; diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index a0a8a513f2..c65442182c 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -225,8 +225,14 @@ const createTestDefinition = ( inputConfig: { inputs: { goal: { type: 'string', required: true, description: 'goal' } }, }, - modelConfig: { model: 'gemini-test-model', temp: 0, top_p: 1 }, - runConfig: { max_time_minutes: 5, max_turns: 5, ...runConfigOverrides }, + modelConfig: { + model: 'gemini-test-model', + generateContentConfig: { + temperature: 0, + topP: 1, + }, + }, + runConfig: { maxTimeMinutes: 5, maxTurns: 5, ...runConfigOverrides }, promptConfig: { systemPrompt: 'Achieve the goal: ${goal}.' }, toolConfig: { tools }, outputConfig, @@ -1321,7 +1327,7 @@ describe('LocalAgentExecutor', () => { it('should terminate when max_turns is reached', async () => { const MAX = 2; const definition = createTestDefinition([LS_TOOL_NAME], { - max_turns: MAX, + maxTurns: MAX, }); const executor = await LocalAgentExecutor.create(definition, mockConfig); @@ -1338,7 +1344,7 @@ describe('LocalAgentExecutor', () => { it('should terminate with TIMEOUT if a model call takes too long', async () => { const definition = createTestDefinition([LS_TOOL_NAME], { - max_time_minutes: 0.5, // 30 seconds + maxTimeMinutes: 0.5, // 30 seconds }); const executor = await LocalAgentExecutor.create( definition, @@ -1395,7 +1401,7 @@ describe('LocalAgentExecutor', () => { it('should terminate with TIMEOUT if a tool call takes too long', async () => { const definition = createTestDefinition([LS_TOOL_NAME], { - max_time_minutes: 1, + maxTimeMinutes: 1, }); const executor = await LocalAgentExecutor.create(definition, mockConfig); @@ -1483,7 +1489,7 @@ describe('LocalAgentExecutor', () => { it('should recover successfully if complete_task is called during the grace turn after MAX_TURNS', async () => { const MAX = 1; const definition = createTestDefinition([LS_TOOL_NAME], { - max_turns: MAX, + maxTurns: MAX, }); const executor = await LocalAgentExecutor.create( definition, @@ -1531,7 +1537,7 @@ describe('LocalAgentExecutor', () => { it('should fail if complete_task is NOT called during the grace turn after MAX_TURNS', async () => { const MAX = 1; const definition = createTestDefinition([LS_TOOL_NAME], { - max_turns: MAX, + maxTurns: MAX, }); const executor = await LocalAgentExecutor.create( definition, @@ -1650,7 +1656,7 @@ describe('LocalAgentExecutor', () => { it('should recover successfully from a TIMEOUT', async () => { const definition = createTestDefinition([LS_TOOL_NAME], { - max_time_minutes: 0.5, // 30 seconds + maxTimeMinutes: 0.5, // 30 seconds }); const executor = await LocalAgentExecutor.create( definition, @@ -1705,7 +1711,7 @@ describe('LocalAgentExecutor', () => { it('should fail recovery from a TIMEOUT if the grace period also times out', async () => { const definition = createTestDefinition([LS_TOOL_NAME], { - max_time_minutes: 0.5, // 30 seconds + maxTimeMinutes: 0.5, // 30 seconds }); const executor = await LocalAgentExecutor.create( definition, @@ -1797,7 +1803,7 @@ describe('LocalAgentExecutor', () => { it('should log a RecoveryAttemptEvent when a recoverable error occurs and recovery fails', async () => { const MAX = 1; const definition = createTestDefinition([LS_TOOL_NAME], { - max_turns: MAX, + maxTurns: MAX, }); const executor = await LocalAgentExecutor.create(definition, mockConfig); @@ -1822,7 +1828,7 @@ describe('LocalAgentExecutor', () => { it('should log a successful RecoveryAttemptEvent when recovery succeeds', async () => { const MAX = 1; const definition = createTestDefinition([LS_TOOL_NAME], { - max_turns: MAX, + maxTurns: MAX, }); const executor = await LocalAgentExecutor.create(definition, mockConfig); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index fc866c97b5..fa5b4701c6 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -361,11 +361,11 @@ export class LocalAgentExecutor { let terminateReason: AgentTerminateMode = AgentTerminateMode.ERROR; let finalResult: string | null = null; - const { max_time_minutes } = this.definition.runConfig; + const { maxTimeMinutes } = this.definition.runConfig; const timeoutController = new AbortController(); const timeoutId = setTimeout( () => timeoutController.abort(new Error('Agent timed out.')), - max_time_minutes * 60 * 1000, + maxTimeMinutes * 60 * 1000, ); // Combine the external signal with the internal timeout signal. @@ -454,13 +454,13 @@ export class LocalAgentExecutor { } else { // Recovery Failed. Set the final error message based on the *original* reason. if (terminateReason === AgentTerminateMode.TIMEOUT) { - finalResult = `Agent timed out after ${this.definition.runConfig.max_time_minutes} minutes.`; + finalResult = `Agent timed out after ${this.definition.runConfig.maxTimeMinutes} minutes.`; this.emitActivity('ERROR', { error: finalResult, context: 'timeout', }); } else if (terminateReason === AgentTerminateMode.MAX_TURNS) { - finalResult = `Agent reached max turns limit (${this.definition.runConfig.max_turns}).`; + finalResult = `Agent reached max turns limit (${this.definition.runConfig.maxTurns}).`; this.emitActivity('ERROR', { error: finalResult, context: 'max_turns', @@ -524,7 +524,7 @@ export class LocalAgentExecutor { } // Recovery failed or wasn't possible - finalResult = `Agent timed out after ${this.definition.runConfig.max_time_minutes} minutes.`; + finalResult = `Agent timed out after ${this.definition.runConfig.maxTimeMinutes} minutes.`; this.emitActivity('ERROR', { error: finalResult, context: 'timeout', @@ -556,7 +556,7 @@ export class LocalAgentExecutor { chat: GeminiChat, prompt_id: string, ): Promise { - const model = this.definition.modelConfig.model; + const model = this.definition.modelConfig.model ?? DEFAULT_GEMINI_MODEL; const { newHistory, info } = await this.compressionService.compress( chat, @@ -1109,7 +1109,7 @@ Important Rules: ): AgentTerminateMode | null { const { runConfig } = this.definition; - if (runConfig.max_turns && turnCounter >= runConfig.max_turns) { + if (runConfig.maxTurns && turnCounter >= runConfig.maxTurns) { return AgentTerminateMode.MAX_TURNS; } diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index 91614cea04..62de4b4c02 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -33,8 +33,14 @@ const testDefinition: LocalAgentDefinition = { priority: { type: 'number', required: false, description: 'prio' }, }, }, - modelConfig: { model: 'test', temp: 0, top_p: 1 }, - runConfig: { max_time_minutes: 1 }, + modelConfig: { + model: 'test', + generateContentConfig: { + temperature: 0, + topP: 1, + }, + }, + runConfig: { maxTimeMinutes: 1 }, promptConfig: { systemPrompt: 'test' }, }; diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 837d4c5f63..a81b3035bb 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -47,8 +47,18 @@ const MOCK_AGENT_V1: AgentDefinition = { name: 'MockAgent', description: 'Mock Description V1', inputConfig: { inputs: {} }, - modelConfig: { model: 'test', temp: 0, top_p: 1 }, - runConfig: { max_time_minutes: 1 }, + modelConfig: { + model: 'test', + generateContentConfig: { + temperature: 0, + topP: 1, + thinkingConfig: { + includeThoughts: true, + thinkingBudget: -1, + }, + }, + }, + runConfig: { maxTimeMinutes: 1 }, promptConfig: { systemPrompt: 'test' }, }; @@ -319,8 +329,8 @@ describe('AgentRegistry', () => { ).toStrictEqual({ model: 'auto', generateContentConfig: { - temperature: autoAgent.modelConfig.temp, - topP: autoAgent.modelConfig.top_p, + temperature: autoAgent.modelConfig.generateContentConfig?.temperature, + topP: autoAgent.modelConfig.generateContentConfig?.topP, thinkingConfig: { includeThoughts: true, thinkingBudget: -1, @@ -352,8 +362,9 @@ describe('AgentRegistry', () => { ).toStrictEqual({ model: MOCK_AGENT_V1.modelConfig.model, generateContentConfig: { - temperature: MOCK_AGENT_V1.modelConfig.temp, - topP: MOCK_AGENT_V1.modelConfig.top_p, + temperature: + MOCK_AGENT_V1.modelConfig.generateContentConfig?.temperature, + topP: MOCK_AGENT_V1.modelConfig.generateContentConfig?.topP, thinkingConfig: { includeThoughts: true, thinkingBudget: -1, @@ -674,9 +685,9 @@ describe('AgentRegistry', () => { await registry.testRegisterAgent(MOCK_AGENT_V1); const def = registry.getDefinition('MockAgent') as LocalAgentDefinition; - expect(def.runConfig.max_turns).toBe(50); - expect(def.runConfig.max_time_minutes).toBe( - MOCK_AGENT_V1.runConfig.max_time_minutes, + expect(def.runConfig.maxTurns).toBe(50); + expect(def.runConfig.maxTimeMinutes).toBe( + MOCK_AGENT_V1.runConfig.maxTimeMinutes, ); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 0038a2b783..49d425bd6e 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -6,8 +6,8 @@ import { Storage } from '../config/storage.js'; import { coreEvents, CoreEvent } from '../utils/events.js'; -import type { Config } from '../config/config.js'; -import type { AgentDefinition } from './types.js'; +import type { AgentOverride, Config } from '../config/config.js'; +import type { AgentDefinition, LocalAgentDefinition } from './types.js'; import { loadAgentsFromDirectory } from './agentLoader.js'; import { CodebaseInvestigatorAgent } from './codebase-investigator.js'; import { CliHelpAgent } from './cli-help-agent.js'; @@ -164,18 +164,26 @@ export class AgentRegistry { modelConfig: { ...CodebaseInvestigatorAgent.modelConfig, model, - thinkingBudget: - investigatorSettings.thinkingBudget ?? - CodebaseInvestigatorAgent.modelConfig.thinkingBudget, + generateContentConfig: { + ...CodebaseInvestigatorAgent.modelConfig.generateContentConfig, + thinkingConfig: { + ...CodebaseInvestigatorAgent.modelConfig.generateContentConfig + ?.thinkingConfig, + thinkingBudget: + investigatorSettings.thinkingBudget ?? + CodebaseInvestigatorAgent.modelConfig.generateContentConfig + ?.thinkingConfig?.thinkingBudget, + }, + }, }, runConfig: { ...CodebaseInvestigatorAgent.runConfig, - max_time_minutes: + maxTimeMinutes: investigatorSettings.maxTimeMinutes ?? - CodebaseInvestigatorAgent.runConfig.max_time_minutes, - max_turns: + CodebaseInvestigatorAgent.runConfig.maxTimeMinutes, + maxTurns: investigatorSettings.maxNumTurns ?? - CodebaseInvestigatorAgent.runConfig.max_turns, + CodebaseInvestigatorAgent.runConfig.maxTurns, }, }; this.registerLocalAgent(agentDef); @@ -229,9 +237,9 @@ export class AgentRegistry { return; } - const overrides = + const settingsOverrides = this.config.getAgentsSettings().overrides?.[definition.name]; - if (overrides?.disabled) { + if (settingsOverrides?.disabled) { if (this.config.getDebugMode()) { debugLogger.log( `[AgentRegistry] Skipping disabled agent '${definition.name}'`, @@ -244,71 +252,10 @@ export class AgentRegistry { debugLogger.log(`[AgentRegistry] Overriding agent '${definition.name}'`); } - // TODO(16443): Refactor definition merging logic into a helper. - // To do this, we need to align the definition of the internal `Definition` - // type with the one exported in settings.json. - const mergedDefinition = { - ...definition, - runConfig: { - ...definition.runConfig, - max_time_minutes: - overrides?.runConfig?.maxTimeMinutes ?? - definition.runConfig.max_time_minutes, - max_turns: - overrides?.runConfig?.maxTurns ?? definition.runConfig.max_turns, - }, - }; - + const mergedDefinition = this.applyOverrides(definition, settingsOverrides); this.agents.set(mergedDefinition.name, mergedDefinition); - // Register model config. We always create a runtime alias. However, - // if the user is using `auto` as a model string then we also create - // runtime overrides to ensure the subagent generation settings are - // respected regardless of the final model string from routing. - // TODO(12916): Migrate sub-agents where possible to static configs. - const modelConfig = mergedDefinition.modelConfig; - let model = modelConfig.model; - if (model === 'inherit') { - model = this.config.getModel(); - } - - let agentModelConfig: ModelConfig = { - model, - generateContentConfig: { - temperature: modelConfig.temp, - topP: modelConfig.top_p, - thinkingConfig: { - includeThoughts: true, - thinkingBudget: modelConfig.thinkingBudget ?? -1, - }, - }, - }; - - // Apply standardized modelConfig overrides if present. - if (overrides?.modelConfig) { - agentModelConfig = ModelConfigService.merge( - agentModelConfig, - overrides.modelConfig, - ); - } - - this.config.modelConfigService.registerRuntimeModelConfig( - getModelConfigAlias(mergedDefinition), - { - modelConfig: agentModelConfig, - }, - ); - - if (agentModelConfig.model && isAutoModel(agentModelConfig.model)) { - this.config.modelConfigService.registerRuntimeModelOverride({ - match: { - overrideScope: mergedDefinition.name, - }, - modelConfig: { - generateContentConfig: agentModelConfig.generateContentConfig, - }, - }); - } + this.registerModelConfigs(mergedDefinition); } /** @@ -376,6 +323,60 @@ export class AgentRegistry { } } + private applyOverrides( + definition: LocalAgentDefinition, + overrides?: AgentOverride, + ): LocalAgentDefinition { + if (definition.kind !== 'local' || !overrides) { + return definition; + } + + return { + ...definition, + runConfig: { + ...definition.runConfig, + ...overrides.runConfig, + }, + modelConfig: ModelConfigService.merge( + definition.modelConfig, + overrides.modelConfig ?? {}, + ), + }; + } + + private registerModelConfigs( + definition: LocalAgentDefinition, + ): void { + const modelConfig = definition.modelConfig; + let model = modelConfig.model; + if (model === 'inherit') { + model = this.config.getModel(); + } + + const agentModelConfig: ModelConfig = { + ...modelConfig, + model, + }; + + this.config.modelConfigService.registerRuntimeModelConfig( + getModelConfigAlias(definition), + { + modelConfig: agentModelConfig, + }, + ); + + if (agentModelConfig.model && isAutoModel(agentModelConfig.model)) { + this.config.modelConfigService.registerRuntimeModelOverride({ + match: { + overrideScope: definition.name, + }, + modelConfig: { + generateContentConfig: agentModelConfig.generateContentConfig, + }, + }); + } + } + /** * Retrieves an agent definition by name. */ diff --git a/packages/core/src/agents/subagent-tool-wrapper.test.ts b/packages/core/src/agents/subagent-tool-wrapper.test.ts index 29a241f32e..c45b085991 100644 --- a/packages/core/src/agents/subagent-tool-wrapper.test.ts +++ b/packages/core/src/agents/subagent-tool-wrapper.test.ts @@ -43,8 +43,14 @@ const mockDefinition: LocalAgentDefinition = { }, }, }, - modelConfig: { model: 'gemini-test-model', temp: 0, top_p: 1 }, - runConfig: { max_time_minutes: 5 }, + modelConfig: { + model: 'gemini-test-model', + generateContentConfig: { + temperature: 0, + topP: 1, + }, + }, + runConfig: { maxTimeMinutes: 5 }, promptConfig: { systemPrompt: 'You are a test agent.' }, }; diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index f0d2743662..23c6f75626 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -11,6 +11,7 @@ import type { Content, FunctionDeclaration } from '@google/genai'; import type { AnyDeclarativeTool } from '../tools/tools.js'; import { type z } from 'zod'; +import type { ModelConfig } from '../services/modelConfigService.js'; /** * Describes the possible termination modes for an agent. @@ -177,22 +178,12 @@ export interface OutputConfig { schema: T; } -/** - * Configures the generative model parameters for the agent. - */ -export interface ModelConfig { - model: string; - temp: number; - top_p: number; - thinkingBudget?: number; -} - /** * Configures the execution environment and constraints for the agent. */ export interface RunConfig { /** The maximum execution time for the agent in minutes. */ - max_time_minutes: number; + maxTimeMinutes: number; /** The maximum number of conversational turns. */ - max_turns?: number; + maxTurns?: number; }