diff --git a/docs/get-started/configuration.md b/docs/get-started/configuration.md index 725e083006..eb30b47165 100644 --- a/docs/get-started/configuration.md +++ b/docs/get-started/configuration.md @@ -784,7 +784,8 @@ their corresponding top-level category object in your `settings.json` file. #### `experimental` - **`experimental.enableAgents`** (boolean): - - **Description:** Enable local and remote subagents. + - **Description:** Enable local and remote subagents. Warning: Experimental + feature, uses YOLO mode for subagents - **Default:** `false` - **Requires restart:** Yes diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index fd4ddf8ff9..ad003ee853 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -352,7 +352,9 @@ describe('SettingsSchema', () => { expect(setting.default).toBe(false); expect(setting.requiresRestart).toBe(true); expect(setting.showInDialog).toBe(false); - expect(setting.description).toBe('Enable local and remote subagents.'); + expect(setting.description).toBe( + 'Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents', + ); }); }); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 8e8347ab15..f8fdae9ca8 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1301,7 +1301,8 @@ const SETTINGS_SCHEMA = { category: 'Experimental', requiresRestart: true, default: false, - description: 'Enable local and remote subagents.', + description: + 'Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents', showInDialog: false, }, extensionManagement: { diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index a1e635df4f..8550ede760 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -301,11 +301,14 @@ describe('LocalAgentExecutor', () => { expect(executor).toBeInstanceOf(LocalAgentExecutor); }); - it('SECURITY: should throw if a tool is not on the non-interactive allowlist', async () => { + it('should allow any tool for experimentation (formerly SECURITY check)', async () => { const definition = createTestDefinition([MOCK_TOOL_NOT_ALLOWED.name]); - await expect( - LocalAgentExecutor.create(definition, mockConfig, onActivity), - ).rejects.toThrow(/not on the allow-list for non-interactive execution/); + const executor = await LocalAgentExecutor.create( + definition, + mockConfig, + onActivity, + ); + expect(executor).toBeInstanceOf(LocalAgentExecutor); }); it('should create an isolated ToolRegistry for the agent', async () => { @@ -605,7 +608,13 @@ describe('LocalAgentExecutor', () => { }); mockModelResponse( - [{ name: TASK_COMPLETE_TOOL_NAME, args: {}, id: 'call2' }], + [ + { + name: TASK_COMPLETE_TOOL_NAME, + args: { result: 'All work done' }, + id: 'call2', + }, + ], 'Task finished.', ); @@ -622,12 +631,12 @@ describe('LocalAgentExecutor', () => { const completeToolDef = sentTools!.find( (t) => t.name === TASK_COMPLETE_TOOL_NAME, ); - expect(completeToolDef?.parameters?.required).toEqual([]); + expect(completeToolDef?.parameters?.required).toEqual(['result']); expect(completeToolDef?.description).toContain( - 'signal that you have completed', + 'submit your final findings', ); - expect(output.result).toBe('Task completed successfully.'); + expect(output.result).toBe('All work done'); expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL); }); @@ -780,8 +789,16 @@ describe('LocalAgentExecutor', () => { // Turn 1: Duplicate calls mockModelResponse([ - { name: TASK_COMPLETE_TOOL_NAME, args: {}, id: 'call1' }, - { name: TASK_COMPLETE_TOOL_NAME, args: {}, id: 'call2' }, + { + name: TASK_COMPLETE_TOOL_NAME, + args: { result: 'done' }, + id: 'call1', + }, + { + name: TASK_COMPLETE_TOOL_NAME, + args: { result: 'ignored' }, + id: 'call2', + }, ]); const output = await executor.run({ goal: 'Dup test' }, signal); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index 52be227bc4..6cb78c5f11 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -20,15 +20,6 @@ import { ToolRegistry } from '../tools/tool-registry.js'; import { type ToolCallRequestInfo, CompressionStatus } from '../core/turn.js'; import { ChatCompressionService } from '../services/chatCompressionService.js'; import { getDirectoryContextString } from '../utils/environmentContext.js'; -import { - GLOB_TOOL_NAME, - GREP_TOOL_NAME, - LS_TOOL_NAME, - MEMORY_TOOL_NAME, - READ_FILE_TOOL_NAME, - READ_MANY_FILES_TOOL_NAME, - WEB_SEARCH_TOOL_NAME, -} from '../tools/tool-names.js'; import { promptIdContext } from '../utils/promptIdContext.js'; import { logAgentStart, @@ -53,6 +44,7 @@ import { type z } from 'zod'; import { zodToJsonSchema } from 'zod-to-json-schema'; import { debugLogger } from '../utils/debugLogger.js'; import { getModelConfigAlias } from './registry.js'; +import { ApprovalMode } from '../policy/types.js'; /** A callback function to report on agent activity. */ export type ActivityCallback = (activity: SubagentActivityEvent) => void; @@ -129,12 +121,6 @@ export class LocalAgentExecutor { } agentToolRegistry.sortTools(); - // Validate that all registered tools are safe for non-interactive - // execution. - await LocalAgentExecutor.validateTools( - agentToolRegistry, - definition.name, - ); } // Get the parent prompt ID from context @@ -802,19 +788,46 @@ export class LocalAgentExecutor { }); } } else { - // No output expected. Just signal completion. - submittedOutput = 'Task completed successfully.'; - syncResponseParts.push({ - functionResponse: { - name: TASK_COMPLETE_TOOL_NAME, - response: { status: 'Task marked complete.' }, - id: callId, - }, - }); - this.emitActivity('TOOL_CALL_END', { - name: functionCall.name, - output: 'Task marked complete.', - }); + // No outputConfig - use default 'result' parameter + const resultArg = args['result']; + if ( + resultArg !== undefined && + resultArg !== null && + resultArg !== '' + ) { + submittedOutput = + typeof resultArg === 'string' + ? resultArg + : JSON.stringify(resultArg, null, 2); + syncResponseParts.push({ + functionResponse: { + name: TASK_COMPLETE_TOOL_NAME, + response: { status: 'Result submitted and task completed.' }, + id: callId, + }, + }); + this.emitActivity('TOOL_CALL_END', { + name: functionCall.name, + output: 'Result submitted and task completed.', + }); + } else { + // No result provided - this is an error for agents expected to return results + taskCompleted = false; // Revoke completion + const error = + 'Missing required "result" argument. You must provide your findings when calling complete_task.'; + syncResponseParts.push({ + functionResponse: { + name: TASK_COMPLETE_TOOL_NAME, + response: { error }, + id: callId, + }, + }); + this.emitActivity('ERROR', { + context: 'tool_call', + name: functionCall.name, + error, + }); + } } continue; } @@ -853,8 +866,18 @@ export class LocalAgentExecutor { // Create a promise for the tool execution const executionPromise = (async () => { + // Force YOLO mode for subagents to prevent hanging on confirmation + const contextProxy = new Proxy(this.runtimeContext, { + get(target, prop, receiver) { + if (prop === 'getApprovalMode') { + return () => ApprovalMode.YOLO; + } + return Reflect.get(target, prop, receiver); + }, + }); + const { response: toolResponse } = await executeToolCall( - this.runtimeContext, + contextProxy, requestInfo, signal, ); @@ -939,7 +962,7 @@ export class LocalAgentExecutor { name: TASK_COMPLETE_TOOL_NAME, description: outputConfig ? 'Call this tool to submit your final answer and complete the task. This is the ONLY way to finish.' - : 'Call this tool to signal that you have completed your task. This is the ONLY way to finish.', + : 'Call this tool to submit your final findings and complete the task. This is the ONLY way to finish.', parameters: { type: Type.OBJECT, properties: {}, @@ -957,6 +980,14 @@ export class LocalAgentExecutor { completeTool.parameters!.properties![outputConfig.outputName] = schema as Schema; completeTool.parameters!.required!.push(outputConfig.outputName); + } else { + completeTool.parameters!.properties!['result'] = { + type: Type.STRING, + description: + 'Your final results or findings to return to the orchestrator. ' + + 'Ensure this is comprehensive and follows any formatting requested in your instructions.', + }; + completeTool.parameters!.required!.push('result'); } toolsList.push(completeTool); @@ -985,10 +1016,19 @@ Important Rules: * Work systematically using available tools to complete your task. * Always use absolute paths for file operations. Construct them using the provided "Environment Context".`; - finalPrompt += ` -* When you have completed your task, you MUST call the \`${TASK_COMPLETE_TOOL_NAME}\` tool. + if (this.definition.outputConfig) { + finalPrompt += ` +* When you have completed your task, you MUST call the \`${TASK_COMPLETE_TOOL_NAME}\` tool with your structured output. * Do not call any other tools in the same turn as \`${TASK_COMPLETE_TOOL_NAME}\`. * This is the ONLY way to complete your mission. If you stop calling tools without calling this, you have failed.`; + } else { + finalPrompt += ` +* When you have completed your task, you MUST call the \`${TASK_COMPLETE_TOOL_NAME}\` tool. +* You MUST include your final findings in the "result" parameter. This is how you return the necessary results for the task to be marked complete. +* Ensure your findings are comprehensive and follow any specific formatting requirements provided in your instructions. +* Do not call any other tools in the same turn as \`${TASK_COMPLETE_TOOL_NAME}\`. +* This is the ONLY way to complete your mission. If you stop calling tools without calling this, you have failed.`; + } return finalPrompt; } @@ -1015,37 +1055,6 @@ Important Rules: }); } - /** - * Validates that all tools in a registry are safe for non-interactive use. - * - * @throws An error if a tool is not on the allow-list for non-interactive execution. - */ - private static async validateTools( - toolRegistry: ToolRegistry, - agentName: string, - ): Promise { - // Tools that are non-interactive. This is temporary until we have tool - // confirmations for subagents. - const allowlist = new Set([ - LS_TOOL_NAME, - READ_FILE_TOOL_NAME, - GREP_TOOL_NAME, - GLOB_TOOL_NAME, - READ_MANY_FILES_TOOL_NAME, - MEMORY_TOOL_NAME, - WEB_SEARCH_TOOL_NAME, - ]); - for (const tool of toolRegistry.getAllTools()) { - if (!allowlist.has(tool.name)) { - throw new Error( - `Tool "${tool.name}" is not on the allow-list for non-interactive ` + - `execution in agent "${agentName}". Only tools that do not require user ` + - `confirmation can be used in subagents.`, - ); - } - } - } - /** * Checks if the agent should terminate due to exceeding configured limits. * diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index f02ac826d8..022e1feb2c 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -10,6 +10,7 @@ import { makeFakeConfig } from '../test-utils/config.js'; import type { AgentDefinition, LocalAgentDefinition } from './types.js'; import type { Config } from '../config/config.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { coreEvents, CoreEvent } from '../utils/events.js'; import { DEFAULT_GEMINI_FLASH_LITE_MODEL, GEMINI_MODEL_ALIAS_AUTO, @@ -17,6 +18,13 @@ import { PREVIEW_GEMINI_MODEL, PREVIEW_GEMINI_MODEL_AUTO, } from '../config/models.js'; +import * as tomlLoader from './toml-loader.js'; + +vi.mock('./toml-loader.js', () => ({ + loadAgentsFromDirectory: vi + .fn() + .mockResolvedValue({ agents: [], errors: [] }), +})); // A test-only subclass to expose the protected `registerAgent` method. class TestableAgentRegistry extends AgentRegistry { @@ -49,6 +57,10 @@ describe('AgentRegistry', () => { // Default configuration (debugMode: false) mockConfig = makeFakeConfig(); registry = new TestableAgentRegistry(mockConfig); + vi.mocked(tomlLoader.loadAgentsFromDirectory).mockResolvedValue({ + agents: [], + errors: [], + }); }); afterEach(() => { @@ -67,7 +79,10 @@ describe('AgentRegistry', () => { // }); it('should log the count of loaded agents in debug mode', async () => { - const debugConfig = makeFakeConfig({ debugMode: true }); + const debugConfig = makeFakeConfig({ + debugMode: true, + enableAgents: true, + }); const debugRegistry = new TestableAgentRegistry(debugConfig); const debugLogSpy = vi .spyOn(debugLogger, 'log') @@ -143,6 +158,60 @@ describe('AgentRegistry', () => { DEFAULT_GEMINI_FLASH_LITE_MODEL, ); }); + + it('should load agents from user and project directories with correct precedence', async () => { + mockConfig = makeFakeConfig({ enableAgents: true }); + registry = new TestableAgentRegistry(mockConfig); + + const userAgent = { + ...MOCK_AGENT_V1, + name: 'common-agent', + description: 'User version', + }; + const projectAgent = { + ...MOCK_AGENT_V1, + name: 'common-agent', + description: 'Project version', + }; + const uniqueProjectAgent = { + ...MOCK_AGENT_V1, + name: 'project-only', + description: 'Project only', + }; + + vi.mocked(tomlLoader.loadAgentsFromDirectory) + .mockResolvedValueOnce({ agents: [userAgent], errors: [] }) // User dir + .mockResolvedValueOnce({ + agents: [projectAgent, uniqueProjectAgent], + errors: [], + }); // Project dir + + await registry.initialize(); + + // Project agent should override user agent + expect(registry.getDefinition('common-agent')?.description).toBe( + 'Project version', + ); + expect(registry.getDefinition('project-only')).toBeDefined(); + expect( + vi.mocked(tomlLoader.loadAgentsFromDirectory), + ).toHaveBeenCalledTimes(2); + }); + + it('should NOT load TOML agents when enableAgents is false', async () => { + const disabledConfig = makeFakeConfig({ + enableAgents: false, + codebaseInvestigatorSettings: { enabled: false }, + }); + const disabledRegistry = new TestableAgentRegistry(disabledConfig); + + await disabledRegistry.initialize(); + + expect(disabledRegistry.getAllDefinitions()).toHaveLength(0); + expect( + vi.mocked(tomlLoader.loadAgentsFromDirectory), + ).not.toHaveBeenCalled(); + }); }); describe('registration logic', () => { @@ -261,6 +330,57 @@ describe('AgentRegistry', () => { }); }); + describe('inheritance and refresh', () => { + it('should resolve "inherit" to the current model from configuration', () => { + const config = makeFakeConfig({ model: 'current-model' }); + const registry = new TestableAgentRegistry(config); + + const agent: AgentDefinition = { + ...MOCK_AGENT_V1, + modelConfig: { ...MOCK_AGENT_V1.modelConfig, model: 'inherit' }, + }; + + registry.testRegisterAgent(agent); + + const resolved = config.modelConfigService.getResolvedConfig({ + model: getModelConfigAlias(agent), + }); + expect(resolved.model).toBe('current-model'); + }); + + it('should update inherited models when the main model changes', async () => { + const config = makeFakeConfig({ model: 'initial-model' }); + const registry = new TestableAgentRegistry(config); + await registry.initialize(); + + const agent: AgentDefinition = { + ...MOCK_AGENT_V1, + name: 'InheritingAgent', + modelConfig: { ...MOCK_AGENT_V1.modelConfig, model: 'inherit' }, + }; + + registry.testRegisterAgent(agent); + + // Verify initial state + let resolved = config.modelConfigService.getResolvedConfig({ + model: getModelConfigAlias(agent), + }); + expect(resolved.model).toBe('initial-model'); + + // Change model and emit event + vi.spyOn(config, 'getModel').mockReturnValue('new-model'); + coreEvents.emit(CoreEvent.ModelChanged, { + model: 'new-model', + }); + + // Verify refreshed state + resolved = config.modelConfigService.getResolvedConfig({ + model: getModelConfigAlias(agent), + }); + expect(resolved.model).toBe('new-model'); + }); + }); + describe('accessors', () => { const ANOTHER_AGENT: AgentDefinition = { ...MOCK_AGENT_V1, diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 72ca625544..d42de545eb 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -4,8 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +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 { loadAgentsFromDirectory } from './toml-loader.js'; import { CodebaseInvestigatorAgent } from './codebase-investigator.js'; import { type z } from 'zod'; import { debugLogger } from '../utils/debugLogger.js'; @@ -16,7 +19,6 @@ import { isPreviewModel, } from '../config/models.js'; import type { ModelConfigAlias } from '../services/modelConfigService.js'; -import { coreEvents, CoreEvent } from '../utils/events.js'; /** * Returns the model config alias for a given agent definition. @@ -44,9 +46,49 @@ export class AgentRegistry { this.loadBuiltInAgents(); coreEvents.on(CoreEvent.ModelChanged, () => { - this.loadBuiltInAgents(); + this.refreshAgents(); }); + if (!this.config.isAgentsEnabled()) { + return; + } + + // Load user-level agents: ~/.gemini/agents/ + const userAgentsDir = Storage.getUserAgentsDir(); + const userAgents = await loadAgentsFromDirectory(userAgentsDir); + for (const error of userAgents.errors) { + debugLogger.warn( + `[AgentRegistry] Error loading user agent: ${error.message}`, + ); + coreEvents.emitFeedback('error', `Agent loading error: ${error.message}`); + } + for (const agent of userAgents.agents) { + this.registerAgent(agent); + } + + // Load project-level agents: .gemini/agents/ (relative to Project Root) + const folderTrustEnabled = this.config.getFolderTrust(); + const isTrustedFolder = this.config.isTrustedFolder(); + + if (!folderTrustEnabled || isTrustedFolder) { + const projectAgentsDir = this.config.storage.getProjectAgentsDir(); + const projectAgents = await loadAgentsFromDirectory(projectAgentsDir); + for (const error of projectAgents.errors) { + coreEvents.emitFeedback( + 'error', + `Agent loading error: ${error.message}`, + ); + } + for (const agent of projectAgents.agents) { + this.registerAgent(agent); + } + } else { + coreEvents.emitFeedback( + 'info', + 'Skipping project agents due to untrusted folder. To enable, ensure that the project root is trusted.', + ); + } + if (this.config.getDebugMode()) { debugLogger.log( `[AgentRegistry] Initialized with ${this.agents.size} agents.`, @@ -95,6 +137,13 @@ export class AgentRegistry { } } + private refreshAgents(): void { + this.loadBuiltInAgents(); + for (const agent of this.agents.values()) { + this.registerAgent(agent); + } + } + /** * Registers an agent definition. If an agent with the same name exists, * it will be overwritten, respecting the precedence established by the @@ -121,10 +170,14 @@ export class AgentRegistry { // TODO(12916): Migrate sub-agents where possible to static configs. if (definition.kind === 'local') { const modelConfig = definition.modelConfig; + let model = modelConfig.model; + if (model === 'inherit') { + model = this.config.getModel(); + } const runtimeAlias: ModelConfigAlias = { modelConfig: { - model: modelConfig.model, + model, generateContentConfig: { temperature: modelConfig.temp, topP: modelConfig.top_p, @@ -181,10 +234,7 @@ export class AgentRegistry { .map(([name, def]) => `- **${name}**: ${def.description}`) .join('\n'); - return `Delegates a task to a specialized sub-agent. - -Available agents: -${agentDescriptions}`; + return `Delegates a task to a specialized sub-agent.\n\nAvailable agents:\n${agentDescriptions}`; } /** diff --git a/packages/core/src/agents/toml-loader.test.ts b/packages/core/src/agents/toml-loader.test.ts new file mode 100644 index 0000000000..68f130a611 --- /dev/null +++ b/packages/core/src/agents/toml-loader.test.ts @@ -0,0 +1,236 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + parseAgentToml, + tomlToAgentDefinition, + loadAgentsFromDirectory, + AgentLoadError, +} from './toml-loader.js'; +import { GEMINI_MODEL_ALIAS_PRO } from '../config/models.js'; +import type { LocalAgentDefinition } from './types.js'; + +describe('toml-loader', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'agent-test-')); + }); + + afterEach(async () => { + if (tempDir) { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + async function writeAgentToml(content: string, fileName = 'test.toml') { + const filePath = path.join(tempDir, fileName); + await fs.writeFile(filePath, content); + return filePath; + } + + describe('parseAgentToml', () => { + it('should parse a valid MVA TOML file', async () => { + const filePath = await writeAgentToml(` + name = "test-agent" + description = "A test agent" + [prompts] + system_prompt = "You are a test agent." + `); + + const result = await parseAgentToml(filePath); + expect(result).toEqual({ + name: 'test-agent', + description: 'A test agent', + prompts: { + system_prompt: 'You are a test agent.', + }, + }); + }); + + it('should throw AgentLoadError if file reading fails', async () => { + const filePath = path.join(tempDir, 'non-existent.toml'); + await expect(parseAgentToml(filePath)).rejects.toThrow(AgentLoadError); + }); + + it('should throw AgentLoadError if TOML parsing fails', async () => { + const filePath = await writeAgentToml('invalid toml ['); + await expect(parseAgentToml(filePath)).rejects.toThrow(AgentLoadError); + }); + + it('should throw AgentLoadError if validation fails (missing required field)', async () => { + const filePath = await writeAgentToml(` + name = "test-agent" + # missing description + [prompts] + system_prompt = "You are a test agent." + `); + await expect(parseAgentToml(filePath)).rejects.toThrow( + /Validation failed/, + ); + }); + + it('should throw AgentLoadError if name is not a slug', async () => { + const filePath = await writeAgentToml(` + name = "Test Agent!" + description = "A test agent" + [prompts] + system_prompt = "You are a test agent." + `); + await expect(parseAgentToml(filePath)).rejects.toThrow( + /Name must be a valid slug/, + ); + }); + + it('should throw AgentLoadError if delegate_to_agent is included in tools', async () => { + const filePath = await writeAgentToml(` + name = "test-agent" + description = "A test agent" + tools = ["run_shell_command", "delegate_to_agent"] + [prompts] + system_prompt = "You are a test agent." + `); + + await expect(parseAgentToml(filePath)).rejects.toThrow( + /tools list cannot include 'delegate_to_agent'/, + ); + }); + + it('should throw AgentLoadError if tools contains invalid names', async () => { + const filePath = await writeAgentToml(` + name = "test-agent" + description = "A test agent" + tools = ["not-a-tool"] + [prompts] + system_prompt = "You are a test agent." + `); + await expect(parseAgentToml(filePath)).rejects.toThrow( + /Validation failed: tools.0: Invalid tool name/, + ); + }); + }); + + describe('tomlToAgentDefinition', () => { + it('should convert valid TOML to AgentDefinition with defaults', () => { + const toml = { + name: 'test-agent', + description: 'A test agent', + prompts: { + system_prompt: 'You are a test agent.', + }, + }; + + const result = tomlToAgentDefinition(toml); + expect(result).toMatchObject({ + name: 'test-agent', + description: 'A test agent', + promptConfig: { + systemPrompt: 'You are a test agent.', + }, + modelConfig: { + model: 'inherit', + top_p: 0.95, + }, + runConfig: { + max_time_minutes: 5, + }, + inputConfig: { + inputs: { + query: { + type: 'string', + required: false, + }, + }, + }, + }); + }); + + it('should pass through model aliases', () => { + const toml = { + name: 'test-agent', + description: 'A test agent', + model: { + model: GEMINI_MODEL_ALIAS_PRO, + }, + prompts: { + system_prompt: 'You are a test agent.', + }, + }; + + const result = tomlToAgentDefinition(toml) as LocalAgentDefinition; + expect(result.modelConfig.model).toBe(GEMINI_MODEL_ALIAS_PRO); + }); + + it('should pass through unknown model names (e.g. auto)', () => { + const toml = { + name: 'test-agent', + description: 'A test agent', + model: { + model: 'auto', + }, + prompts: { + system_prompt: 'You are a test agent.', + }, + }; + + const result = tomlToAgentDefinition(toml) as LocalAgentDefinition; + expect(result.modelConfig.model).toBe('auto'); + }); + }); + + describe('loadAgentsFromDirectory', () => { + it('should load definitions from a directory', async () => { + await writeAgentToml( + ` + name = "agent-1" + description = "Agent 1" + [prompts] + system_prompt = "Prompt 1" + `, + 'valid.toml', + ); + + // Create a non-TOML file + await fs.writeFile(path.join(tempDir, 'other.txt'), 'content'); + + // Create a hidden file + await writeAgentToml( + ` + name = "hidden" + description = "Hidden" + [prompts] + system_prompt = "Hidden" + `, + '_hidden.toml', + ); + + const result = await loadAgentsFromDirectory(tempDir); + expect(result.agents).toHaveLength(1); + expect(result.agents[0].name).toBe('agent-1'); + expect(result.errors).toHaveLength(0); + }); + + it('should return empty result if directory does not exist', async () => { + const nonExistentDir = path.join(tempDir, 'does-not-exist'); + const result = await loadAgentsFromDirectory(nonExistentDir); + expect(result.agents).toHaveLength(0); + expect(result.errors).toHaveLength(0); + }); + + it('should capture errors for malformed individual files', async () => { + // Create a malformed TOML file + await writeAgentToml('invalid toml [', 'malformed.toml'); + + const result = await loadAgentsFromDirectory(tempDir); + expect(result.agents).toHaveLength(0); + expect(result.errors).toHaveLength(1); + }); + }); +}); diff --git a/packages/core/src/agents/toml-loader.ts b/packages/core/src/agents/toml-loader.ts new file mode 100644 index 0000000000..973854955c --- /dev/null +++ b/packages/core/src/agents/toml-loader.ts @@ -0,0 +1,251 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import TOML from '@iarna/toml'; +import * as fs from 'node:fs/promises'; +import { type Dirent } from 'node:fs'; +import * as path from 'node:path'; +import { z } from 'zod'; +import type { AgentDefinition } from './types.js'; +import { + isValidToolName, + DELEGATE_TO_AGENT_TOOL_NAME, +} from '../tools/tool-names.js'; + +/** + * DTO for TOML parsing - represents the raw structure of the TOML file. + */ +export interface TomlAgentDefinition { + name: string; + description: string; + display_name?: string; + tools?: string[]; + prompts: { + system_prompt: string; + query?: string; + }; + model?: { + model?: string; + temperature?: number; + }; + run?: { + max_turns?: number; + timeout_mins?: number; + }; +} + +/** + * Error thrown when an agent definition is invalid or cannot be loaded. + */ +export class AgentLoadError extends Error { + constructor( + public filePath: string, + message: string, + ) { + super(`Failed to load agent from ${filePath}: ${message}`); + this.name = 'AgentLoadError'; + } +} + +/** + * Result of loading agents from a directory. + */ +export interface AgentLoadResult { + agents: AgentDefinition[]; + errors: AgentLoadError[]; +} + +const tomlSchema = z.object({ + name: z.string().regex(/^[a-z0-9-_]+$/, 'Name must be a valid slug'), + description: z.string().min(1), + display_name: z.string().optional(), + tools: z + .array( + z.string().refine((val) => isValidToolName(val), { + message: 'Invalid tool name', + }), + ) + .optional(), + prompts: z.object({ + system_prompt: z.string().min(1), + query: z.string().optional(), + }), + model: z + .object({ + model: z.string().optional(), + temperature: z.number().optional(), + }) + .optional(), + run: z + .object({ + max_turns: z.number().int().positive().optional(), + timeout_mins: z.number().int().positive().optional(), + }) + .optional(), +}); + +/** + * Parses and validates an agent TOML file. + * + * @param filePath Path to the TOML file. + * @returns The parsed and validated TomlAgentDefinition. + * @throws AgentLoadError if parsing or validation fails. + */ +export async function parseAgentToml( + filePath: string, +): Promise { + let content: string; + try { + content = await fs.readFile(filePath, 'utf-8'); + } catch (error) { + throw new AgentLoadError( + filePath, + `Could not read file: ${(error as Error).message}`, + ); + } + + let raw: unknown; + try { + raw = TOML.parse(content); + } catch (error) { + throw new AgentLoadError( + filePath, + `TOML parsing failed: ${(error as Error).message}`, + ); + } + + const result = tomlSchema.safeParse(raw); + if (!result.success) { + const issues = result.error.issues + .map((i) => `${i.path.join('.')}: ${i.message}`) + .join(', '); + throw new AgentLoadError(filePath, `Validation failed: ${issues}`); + } + + const definition = result.data as TomlAgentDefinition; + + // Prevent sub-agents from delegating to other agents (to prevent recursion/complexity) + if (definition.tools?.includes(DELEGATE_TO_AGENT_TOOL_NAME)) { + throw new AgentLoadError( + filePath, + `Validation failed: tools list cannot include '${DELEGATE_TO_AGENT_TOOL_NAME}'. Sub-agents cannot delegate to other agents.`, + ); + } + + return definition; +} + +/** + * Converts a TomlAgentDefinition DTO to the internal AgentDefinition structure. + * + * @param toml The parsed TOML definition. + * @returns The internal AgentDefinition. + */ +export function tomlToAgentDefinition( + toml: TomlAgentDefinition, +): AgentDefinition { + // If a model is specified, use it. Otherwise, inherit + const modelName = toml.model?.model || 'inherit'; + + return { + kind: 'local', + name: toml.name, + description: toml.description, + displayName: toml.display_name, + promptConfig: { + systemPrompt: toml.prompts.system_prompt, + query: toml.prompts.query, + }, + modelConfig: { + model: modelName, + temp: toml.model?.temperature ?? 1, + top_p: 0.95, + }, + runConfig: { + max_turns: toml.run?.max_turns, + max_time_minutes: toml.run?.timeout_mins || 5, + }, + toolConfig: toml.tools + ? { + tools: toml.tools, + } + : undefined, + // Default input config for MVA + inputConfig: { + inputs: { + query: { + type: 'string', + description: 'The task for the agent.', + required: false, + }, + }, + }, + }; +} + +/** + * Loads all agents from a specific directory. + * Ignores non-TOML files and files starting with _. + * + * @param dir Directory path to scan. + * @returns Object containing successfully loaded agents and any errors. + */ +export async function loadAgentsFromDirectory( + dir: string, +): Promise { + const result: AgentLoadResult = { + agents: [], + errors: [], + }; + + let dirEntries: Dirent[]; + try { + dirEntries = await fs.readdir(dir, { withFileTypes: true }); + } catch (error) { + // If directory doesn't exist, just return empty + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return result; + } + result.errors.push( + new AgentLoadError( + dir, + `Could not list directory: ${(error as Error).message}`, + ), + ); + return result; + } + + const files = dirEntries + .filter( + (entry) => + entry.isFile() && + entry.name.endsWith('.toml') && + !entry.name.startsWith('_'), + ) + .map((entry) => entry.name); + + for (const file of files) { + const filePath = path.join(dir, file); + try { + const toml = await parseAgentToml(filePath); + const agent = tomlToAgentDefinition(toml); + result.agents.push(agent); + } catch (error) { + if (error instanceof AgentLoadError) { + result.errors.push(error); + } else { + result.errors.push( + new AgentLoadError( + filePath, + `Unexpected error: ${(error as Error).message}`, + ), + ); + } + } + } + + return result; +} diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 075eb9fb16..fedf2f753f 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -45,6 +45,16 @@ describe('Storage – additional helpers', () => { expect(storage.getProjectCommandsDir()).toBe(expected); }); + it('getUserAgentsDir returns ~/.gemini/agents', () => { + const expected = path.join(os.homedir(), GEMINI_DIR, 'agents'); + expect(Storage.getUserAgentsDir()).toBe(expected); + }); + + it('getProjectAgentsDir returns project/.gemini/agents', () => { + const expected = path.join(projectRoot, GEMINI_DIR, 'agents'); + expect(storage.getProjectAgentsDir()).toBe(expected); + }); + it('getMcpOAuthTokensPath returns ~/.gemini/mcp-oauth-tokens.json', () => { const expected = path.join( os.homedir(), diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index a98030c49d..d5c32ae8db 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -58,6 +58,10 @@ export class Storage { return path.join(Storage.getGlobalGeminiDir(), 'policies'); } + static getUserAgentsDir(): string { + return path.join(Storage.getGlobalGeminiDir(), 'agents'); + } + static getSystemSettingsPath(): string { if (process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']) { return process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH']; @@ -123,6 +127,10 @@ export class Storage { return path.join(this.getGeminiDir(), 'commands'); } + getProjectAgentsDir(): string { + return path.join(this.getGeminiDir(), 'agents'); + } + getProjectTempCheckpointsDir(): string { return path.join(this.getProjectTempDir(), 'checkpoints'); } diff --git a/packages/core/src/tools/tool-names.test.ts b/packages/core/src/tools/tool-names.test.ts new file mode 100644 index 0000000000..805727c283 --- /dev/null +++ b/packages/core/src/tools/tool-names.test.ts @@ -0,0 +1,57 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + isValidToolName, + ALL_BUILTIN_TOOL_NAMES, + DISCOVERED_TOOL_PREFIX, + LS_TOOL_NAME, +} from './tool-names.js'; + +describe('tool-names', () => { + describe('isValidToolName', () => { + it('should validate built-in tool names', () => { + expect(isValidToolName(LS_TOOL_NAME)).toBe(true); + for (const name of ALL_BUILTIN_TOOL_NAMES) { + expect(isValidToolName(name)).toBe(true); + } + }); + + it('should validate discovered tool names', () => { + expect(isValidToolName(`${DISCOVERED_TOOL_PREFIX}my_tool`)).toBe(true); + }); + + it('should validate MCP tool names (server__tool)', () => { + expect(isValidToolName('server__tool')).toBe(true); + expect(isValidToolName('my-server__my-tool')).toBe(true); + }); + + it('should reject invalid tool names', () => { + expect(isValidToolName('')).toBe(false); + expect(isValidToolName('invalid-name')).toBe(false); + expect(isValidToolName('server__')).toBe(false); + expect(isValidToolName('__tool')).toBe(false); + expect(isValidToolName('server__tool__extra')).toBe(false); + }); + + it('should handle wildcards when allowed', () => { + // Default: not allowed + expect(isValidToolName('*')).toBe(false); + expect(isValidToolName('server__*')).toBe(false); + + // Explicitly allowed + expect(isValidToolName('*', { allowWildcards: true })).toBe(true); + expect(isValidToolName('server__*', { allowWildcards: true })).toBe(true); + + // Invalid wildcards + expect(isValidToolName('__*', { allowWildcards: true })).toBe(false); + expect(isValidToolName('server__tool*', { allowWildcards: true })).toBe( + false, + ); + }); + }); +}); diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index b76b429000..cec12de72c 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -22,3 +22,70 @@ export const LS_TOOL_NAME = 'list_directory'; export const MEMORY_TOOL_NAME = 'save_memory'; export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); export const DELEGATE_TO_AGENT_TOOL_NAME = 'delegate_to_agent'; + +/** Prefix used for tools discovered via the toolDiscoveryCommand. */ +export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_'; + +/** + * List of all built-in tool names. + */ +export const ALL_BUILTIN_TOOL_NAMES = [ + GLOB_TOOL_NAME, + WRITE_TODOS_TOOL_NAME, + WRITE_FILE_TOOL_NAME, + WEB_SEARCH_TOOL_NAME, + WEB_FETCH_TOOL_NAME, + EDIT_TOOL_NAME, + SHELL_TOOL_NAME, + GREP_TOOL_NAME, + READ_MANY_FILES_TOOL_NAME, + READ_FILE_TOOL_NAME, + LS_TOOL_NAME, + MEMORY_TOOL_NAME, + DELEGATE_TO_AGENT_TOOL_NAME, +] as const; + +/** + * Validates if a tool name is syntactically valid. + * Checks against built-in tools, discovered tools, and MCP naming conventions. + */ +export function isValidToolName( + name: string, + options: { allowWildcards?: boolean } = {}, +): boolean { + // Built-in tools + if ((ALL_BUILTIN_TOOL_NAMES as readonly string[]).includes(name)) { + return true; + } + + // Discovered tools + if (name.startsWith(DISCOVERED_TOOL_PREFIX)) { + return true; + } + + // Policy wildcards + if (options.allowWildcards && name === '*') { + return true; + } + + // MCP tools (format: server__tool) + if (name.includes('__')) { + const parts = name.split('__'); + if (parts.length !== 2 || parts[0].length === 0 || parts[1].length === 0) { + return false; + } + + const server = parts[0]; + const tool = parts[1]; + + if (tool === '*') { + return !!options.allowWildcards; + } + + // Basic slug validation for server and tool names + const slugRegex = /^[a-z0-9-_]+$/i; + return slugRegex.test(server) && slugRegex.test(tool); + } + + return false; +} diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 19ab2643d9..55eb89150a 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -11,11 +11,8 @@ import type { ConfigParameters } from '../config/config.js'; import { Config } from '../config/config.js'; import { ApprovalMode } from '../policy/types.js'; -import { - ToolRegistry, - DiscoveredTool, - DISCOVERED_TOOL_PREFIX, -} from './tool-registry.js'; +import { ToolRegistry, DiscoveredTool } from './tool-registry.js'; +import { DISCOVERED_TOOL_PREFIX } from './tool-names.js'; import { DiscoveredMCPTool } from './mcp-tool.js'; import type { FunctionDeclaration, CallableTool } from '@google/genai'; import { mcpToTool } from '@google/genai'; diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index c59e82e932..535bed7dea 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -21,8 +21,7 @@ import { safeJsonStringify } from '../utils/safeJsonStringify.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents } from '../utils/events.js'; - -export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_'; +import { DISCOVERED_TOOL_PREFIX } from './tool-names.js'; type ToolParams = Record; diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 58ad1b78f9..bcec7eaaf1 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1303,8 +1303,8 @@ "properties": { "enableAgents": { "title": "Enable Agents", - "description": "Enable local and remote subagents.", - "markdownDescription": "Enable local and remote subagents.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`", + "description": "Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents", + "markdownDescription": "Enable local and remote subagents. Warning: Experimental feature, uses YOLO mode for subagents\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`", "default": false, "type": "boolean" },