mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-01 19:03:42 +00:00
Refactor subagent delegation to be one tool per agent (#17346)
This commit is contained in:
committed by
GitHub
parent
07f8299087
commit
954bc41041
@@ -111,19 +111,6 @@ Body`);
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw AgentLoadError if tools list includes forbidden tool', async () => {
|
||||
const filePath = await writeAgentMarkdown(`---
|
||||
name: test-agent
|
||||
description: Test
|
||||
tools:
|
||||
- delegate_to_agent
|
||||
---
|
||||
Body`);
|
||||
await expect(parseAgentMarkdown(filePath)).rejects.toThrow(
|
||||
/tools list cannot include 'delegate_to_agent'/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should parse a valid remote agent markdown file', async () => {
|
||||
const filePath = await writeAgentMarkdown(`---
|
||||
kind: remote
|
||||
|
||||
@@ -10,10 +10,7 @@ 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';
|
||||
import { isValidToolName } from '../tools/tool-names.js';
|
||||
import { FRONTMATTER_REGEX } from '../skills/skillLoader.js';
|
||||
|
||||
/**
|
||||
@@ -217,15 +214,6 @@ export async function parseAgentMarkdown(
|
||||
|
||||
// Local agent validation
|
||||
// Validate tools
|
||||
if (
|
||||
frontmatter.tools &&
|
||||
frontmatter.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.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Construct the local agent definition
|
||||
const agentDef: FrontmatterLocalAgentDefinition = {
|
||||
|
||||
@@ -1,346 +0,0 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import {
|
||||
DelegateToAgentTool,
|
||||
type DelegateParams,
|
||||
} from './delegate-to-agent-tool.js';
|
||||
import { AgentRegistry } from './registry.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { AgentDefinition } from './types.js';
|
||||
import { LocalSubagentInvocation } from './local-invocation.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import { MessageBusType } from '../confirmation-bus/types.js';
|
||||
import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js';
|
||||
import { RemoteAgentInvocation } from './remote-invocation.js';
|
||||
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
|
||||
|
||||
vi.mock('./local-invocation.js', () => ({
|
||||
LocalSubagentInvocation: vi.fn().mockImplementation(() => ({
|
||||
execute: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ content: [{ type: 'text', text: 'Success' }] }),
|
||||
shouldConfirmExecute: vi.fn().mockResolvedValue(false),
|
||||
})),
|
||||
}));
|
||||
|
||||
vi.mock('./remote-invocation.js', () => ({
|
||||
RemoteAgentInvocation: vi.fn().mockImplementation(() => ({
|
||||
execute: vi.fn().mockResolvedValue({
|
||||
content: [{ type: 'text', text: 'Remote Success' }],
|
||||
}),
|
||||
shouldConfirmExecute: vi.fn().mockResolvedValue({
|
||||
type: 'info',
|
||||
title: 'Remote Confirmation',
|
||||
prompt: 'Confirm remote call',
|
||||
onConfirm: vi.fn(),
|
||||
}),
|
||||
})),
|
||||
}));
|
||||
|
||||
describe('DelegateToAgentTool', () => {
|
||||
let registry: AgentRegistry;
|
||||
let config: Config;
|
||||
let tool: DelegateToAgentTool;
|
||||
let messageBus: MessageBus;
|
||||
|
||||
const mockAgentDef: AgentDefinition = {
|
||||
kind: 'local',
|
||||
name: 'test_agent',
|
||||
description: 'A test agent',
|
||||
promptConfig: {},
|
||||
modelConfig: {
|
||||
model: 'test-model',
|
||||
generateContentConfig: {
|
||||
temperature: 0,
|
||||
topP: 0,
|
||||
},
|
||||
},
|
||||
inputConfig: {
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
arg1: { type: 'string', description: 'Argument 1' },
|
||||
arg2: { type: 'number', description: 'Argument 2' },
|
||||
},
|
||||
required: ['arg1'],
|
||||
},
|
||||
},
|
||||
runConfig: { maxTurns: 1, maxTimeMinutes: 1 },
|
||||
toolConfig: { tools: [] },
|
||||
};
|
||||
|
||||
const mockRemoteAgentDef: AgentDefinition = {
|
||||
kind: 'remote',
|
||||
name: 'remote_agent',
|
||||
description: 'A remote agent',
|
||||
agentCardUrl: 'https://example.com/agent.json',
|
||||
inputConfig: {
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
query: { type: 'string', description: 'Query' },
|
||||
},
|
||||
required: ['query'],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
config = {
|
||||
getDebugMode: () => false,
|
||||
getActiveModel: () => 'test-model',
|
||||
modelConfigService: {
|
||||
registerRuntimeModelConfig: vi.fn(),
|
||||
},
|
||||
} as unknown as Config;
|
||||
|
||||
registry = new AgentRegistry(config);
|
||||
// Manually register the mock agent (bypassing protected method for testing)
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(registry as any).agents.set(mockAgentDef.name, mockAgentDef);
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(registry as any).agents.set(mockRemoteAgentDef.name, mockRemoteAgentDef);
|
||||
|
||||
messageBus = createMockMessageBus();
|
||||
|
||||
tool = new DelegateToAgentTool(registry, config, messageBus);
|
||||
});
|
||||
|
||||
it('should use dynamic description from registry', () => {
|
||||
// registry has mockAgentDef registered in beforeEach
|
||||
expect(tool.description).toContain(
|
||||
'Delegates a task to a specialized sub-agent',
|
||||
);
|
||||
expect(tool.description).toContain(
|
||||
`- **${mockAgentDef.name}**: ${mockAgentDef.description}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw helpful error when agent_name does not exist', async () => {
|
||||
// We allow validation to pass now, checking happens in execute.
|
||||
const invocation = tool.build({
|
||||
agent_name: 'non_existent_agent',
|
||||
} as DelegateParams);
|
||||
|
||||
await expect(() =>
|
||||
invocation.execute(new AbortController().signal),
|
||||
).rejects.toThrow(
|
||||
"Agent 'non_existent_agent' not found. Available agents are: 'test_agent' (A test agent), 'remote_agent' (A remote agent). Please choose a valid agent_name.",
|
||||
);
|
||||
});
|
||||
|
||||
it('should validate correct arguments', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg1: 'valid',
|
||||
});
|
||||
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
expect(result).toEqual({ content: [{ type: 'text', text: 'Success' }] });
|
||||
expect(LocalSubagentInvocation).toHaveBeenCalledWith(
|
||||
mockAgentDef,
|
||||
config,
|
||||
{ arg1: 'valid' },
|
||||
messageBus,
|
||||
mockAgentDef.name,
|
||||
mockAgentDef.name,
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw helpful error for missing required argument', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg2: 123,
|
||||
} as DelegateParams);
|
||||
|
||||
await expect(() =>
|
||||
invocation.execute(new AbortController().signal),
|
||||
).rejects.toThrow(
|
||||
`Invalid arguments for agent 'test_agent': params must have required property 'arg1'. Input schema: ${JSON.stringify(mockAgentDef.inputConfig.inputSchema)}.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw helpful error for invalid argument type', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg1: 123,
|
||||
} as DelegateParams);
|
||||
|
||||
await expect(() =>
|
||||
invocation.execute(new AbortController().signal),
|
||||
).rejects.toThrow(
|
||||
`Invalid arguments for agent 'test_agent': params/arg1 must be string. Input schema: ${JSON.stringify(mockAgentDef.inputConfig.inputSchema)}.`,
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow optional arguments to be omitted', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg1: 'valid',
|
||||
// arg2 is optional
|
||||
});
|
||||
|
||||
await expect(
|
||||
invocation.execute(new AbortController().signal),
|
||||
).resolves.toBeDefined();
|
||||
});
|
||||
|
||||
it('should throw error if an agent has an input named "agent_name"', () => {
|
||||
const invalidAgentDef: AgentDefinition = {
|
||||
...mockAgentDef,
|
||||
name: 'invalid_agent',
|
||||
inputConfig: {
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
agent_name: {
|
||||
type: 'string',
|
||||
description: 'Conflict',
|
||||
},
|
||||
},
|
||||
required: ['agent_name'],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(registry as any).agents.set(invalidAgentDef.name, invalidAgentDef);
|
||||
|
||||
expect(() => new DelegateToAgentTool(registry, config, messageBus)).toThrow(
|
||||
"Agent 'invalid_agent' cannot have an input parameter named 'agent_name' as it is a reserved parameter for delegation.",
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow a remote agent missing a "query" input (will default at runtime)', () => {
|
||||
const invalidRemoteAgentDef: AgentDefinition = {
|
||||
kind: 'remote',
|
||||
name: 'invalid_remote',
|
||||
description: 'Conflict',
|
||||
agentCardUrl: 'https://example.com/agent.json',
|
||||
inputConfig: {
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
not_query: {
|
||||
type: 'string',
|
||||
description: 'Not a query',
|
||||
},
|
||||
},
|
||||
required: ['not_query'],
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(registry as any).agents.set(
|
||||
invalidRemoteAgentDef.name,
|
||||
invalidRemoteAgentDef,
|
||||
);
|
||||
|
||||
expect(
|
||||
() => new DelegateToAgentTool(registry, config, messageBus),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should execute local agents silently without requesting confirmation', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg1: 'valid',
|
||||
});
|
||||
|
||||
// Trigger confirmation check
|
||||
const result = await invocation.shouldConfirmExecute(
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
expect(result).toBe(false);
|
||||
|
||||
// Verify it did NOT call messageBus.publish with 'delegate_to_agent'
|
||||
const delegateToAgentPublish = vi
|
||||
.mocked(messageBus.publish)
|
||||
.mock.calls.find(
|
||||
(call) =>
|
||||
call[0].type === MessageBusType.TOOL_CONFIRMATION_REQUEST &&
|
||||
call[0].toolCall.name === DELEGATE_TO_AGENT_TOOL_NAME,
|
||||
);
|
||||
expect(delegateToAgentPublish).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should delegate to remote agent correctly', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'remote_agent',
|
||||
query: 'hello remote',
|
||||
});
|
||||
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
expect(result).toEqual({
|
||||
content: [{ type: 'text', text: 'Remote Success' }],
|
||||
});
|
||||
expect(RemoteAgentInvocation).toHaveBeenCalledWith(
|
||||
mockRemoteAgentDef,
|
||||
{ query: 'hello remote' },
|
||||
messageBus,
|
||||
'remote_agent',
|
||||
'remote_agent',
|
||||
);
|
||||
});
|
||||
|
||||
describe('Confirmation', () => {
|
||||
it('should return false for local agents (silent execution)', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'test_agent',
|
||||
arg1: 'valid',
|
||||
});
|
||||
|
||||
// Local agents should now return false directly, bypassing policy check
|
||||
const result = await invocation.shouldConfirmExecute(
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
expect(result).toBe(false);
|
||||
|
||||
const delegateToAgentPublish = vi
|
||||
.mocked(messageBus.publish)
|
||||
.mock.calls.find(
|
||||
(call) =>
|
||||
call[0].type === MessageBusType.TOOL_CONFIRMATION_REQUEST &&
|
||||
call[0].toolCall.name === DELEGATE_TO_AGENT_TOOL_NAME,
|
||||
);
|
||||
expect(delegateToAgentPublish).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should forward to remote agent confirmation logic', async () => {
|
||||
const invocation = tool.build({
|
||||
agent_name: 'remote_agent',
|
||||
query: 'hello remote',
|
||||
});
|
||||
|
||||
const result = await invocation.shouldConfirmExecute(
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
// Verify it returns the mock confirmation from RemoteAgentInvocation
|
||||
expect(result).toMatchObject({
|
||||
type: 'info',
|
||||
title: 'Remote Confirmation',
|
||||
});
|
||||
|
||||
// Verify it did NOT call messageBus.publish with 'delegate_to_agent'
|
||||
// directly from DelegateInvocation, but instead went into RemoteAgentInvocation.
|
||||
// RemoteAgentInvocation (the mock) doesn't call publish in its mock implementation.
|
||||
const delegateToAgentPublish = vi
|
||||
.mocked(messageBus.publish)
|
||||
.mock.calls.find(
|
||||
(call) =>
|
||||
call[0].type === MessageBusType.TOOL_CONFIRMATION_REQUEST &&
|
||||
call[0].toolCall.name === DELEGATE_TO_AGENT_TOOL_NAME,
|
||||
);
|
||||
expect(delegateToAgentPublish).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,240 +0,0 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import {
|
||||
BaseDeclarativeTool,
|
||||
Kind,
|
||||
type ToolInvocation,
|
||||
type ToolResult,
|
||||
BaseToolInvocation,
|
||||
type ToolCallConfirmationDetails,
|
||||
} from '../tools/tools.js';
|
||||
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
||||
import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js';
|
||||
import type { AgentRegistry } from './registry.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import type { AgentDefinition, AgentInputs } from './types.js';
|
||||
import { SubagentToolWrapper } from './subagent-tool-wrapper.js';
|
||||
import { SchemaValidator } from '../utils/schemaValidator.js';
|
||||
import { type AnySchema } from 'ajv';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
|
||||
export type DelegateParams = { agent_name: string } & Record<string, unknown>;
|
||||
|
||||
export class DelegateToAgentTool extends BaseDeclarativeTool<
|
||||
DelegateParams,
|
||||
ToolResult
|
||||
> {
|
||||
constructor(
|
||||
private readonly registry: AgentRegistry,
|
||||
private readonly config: Config,
|
||||
messageBus: MessageBus,
|
||||
) {
|
||||
const definitions = registry.getAllDefinitions();
|
||||
|
||||
let toolSchema: AnySchema;
|
||||
|
||||
if (definitions.length === 0) {
|
||||
// Fallback if no agents are registered (mostly for testing/safety)
|
||||
toolSchema = {
|
||||
type: 'object',
|
||||
properties: {
|
||||
agent_name: {
|
||||
type: 'string',
|
||||
description: 'No agents are currently available.',
|
||||
},
|
||||
},
|
||||
required: ['agent_name'],
|
||||
};
|
||||
} else {
|
||||
const agentSchemas = definitions.map((def) => {
|
||||
const schemaError = SchemaValidator.validateSchema(
|
||||
def.inputConfig.inputSchema,
|
||||
);
|
||||
if (schemaError) {
|
||||
throw new Error(`Invalid schema for ${def.name}: ${schemaError}`);
|
||||
}
|
||||
|
||||
const inputSchema = def.inputConfig.inputSchema;
|
||||
if (typeof inputSchema !== 'object' || inputSchema === null) {
|
||||
throw new Error(`Agent '${def.name}' must provide an object schema.`);
|
||||
}
|
||||
|
||||
const schemaObj = inputSchema as Record<string, unknown>;
|
||||
const properties = schemaObj['properties'] as
|
||||
| Record<string, unknown>
|
||||
| undefined;
|
||||
if (properties && 'agent_name' in properties) {
|
||||
throw new Error(
|
||||
`Agent '${def.name}' cannot have an input parameter named 'agent_name' as it is a reserved parameter for delegation.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (def.kind === 'remote') {
|
||||
if (!properties || !properties['query']) {
|
||||
debugLogger.log(
|
||||
'INFO',
|
||||
`Remote agent '${def.name}' does not define a 'query' property in its inputSchema. It will default to 'Get Started!' during invocation.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
type: 'object',
|
||||
properties: {
|
||||
agent_name: {
|
||||
const: def.name,
|
||||
description: def.description,
|
||||
},
|
||||
...(properties || {}),
|
||||
},
|
||||
required: [
|
||||
'agent_name',
|
||||
...((schemaObj['required'] as string[]) || []),
|
||||
],
|
||||
} as AnySchema;
|
||||
});
|
||||
|
||||
// Create the anyOf schema
|
||||
if (agentSchemas.length === 1) {
|
||||
toolSchema = agentSchemas[0];
|
||||
} else {
|
||||
toolSchema = {
|
||||
anyOf: agentSchemas,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
super(
|
||||
DELEGATE_TO_AGENT_TOOL_NAME,
|
||||
'Delegate to Agent',
|
||||
registry.getToolDescription(),
|
||||
Kind.Think,
|
||||
toolSchema,
|
||||
messageBus,
|
||||
/* isOutputMarkdown */ true,
|
||||
/* canUpdateOutput */ true,
|
||||
);
|
||||
}
|
||||
|
||||
override validateToolParams(_params: DelegateParams): string | null {
|
||||
// We override the default schema validation because the generic JSON schema validation
|
||||
// produces poor error messages for discriminated unions (anyOf).
|
||||
// Instead, we perform detailed, agent-specific validation in the `execute` method
|
||||
// to provide rich error messages that help the LLM self-heal.
|
||||
return null;
|
||||
}
|
||||
|
||||
protected createInvocation(
|
||||
params: DelegateParams,
|
||||
messageBus: MessageBus,
|
||||
_toolName?: string,
|
||||
_toolDisplayName?: string,
|
||||
): ToolInvocation<DelegateParams, ToolResult> {
|
||||
return new DelegateInvocation(
|
||||
params,
|
||||
this.registry,
|
||||
this.config,
|
||||
messageBus,
|
||||
_toolName,
|
||||
_toolDisplayName,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
class DelegateInvocation extends BaseToolInvocation<
|
||||
DelegateParams,
|
||||
ToolResult
|
||||
> {
|
||||
constructor(
|
||||
params: DelegateParams,
|
||||
private readonly registry: AgentRegistry,
|
||||
private readonly config: Config,
|
||||
messageBus: MessageBus,
|
||||
_toolName?: string,
|
||||
_toolDisplayName?: string,
|
||||
) {
|
||||
super(
|
||||
params,
|
||||
messageBus,
|
||||
_toolName ?? DELEGATE_TO_AGENT_TOOL_NAME,
|
||||
_toolDisplayName,
|
||||
);
|
||||
}
|
||||
|
||||
getDescription(): string {
|
||||
return `Delegating to agent '${this.params.agent_name}'`;
|
||||
}
|
||||
|
||||
override async shouldConfirmExecute(
|
||||
abortSignal: AbortSignal,
|
||||
): Promise<ToolCallConfirmationDetails | false> {
|
||||
const definition = this.registry.getDefinition(this.params.agent_name);
|
||||
if (!definition || definition.kind !== 'remote') {
|
||||
// Local agents should execute without confirmation. Inner tool calls will bubble up their own confirmations to the user.
|
||||
return false;
|
||||
}
|
||||
|
||||
const { agent_name: _agent_name, ...agentArgs } = this.params;
|
||||
const invocation = this.buildSubInvocation(
|
||||
definition,
|
||||
agentArgs as AgentInputs,
|
||||
);
|
||||
return invocation.shouldConfirmExecute(abortSignal);
|
||||
}
|
||||
|
||||
async execute(
|
||||
signal: AbortSignal,
|
||||
updateOutput?: (output: string | AnsiOutput) => void,
|
||||
): Promise<ToolResult> {
|
||||
const definition = this.registry.getDefinition(this.params.agent_name);
|
||||
if (!definition) {
|
||||
const availableAgents = this.registry
|
||||
.getAllDefinitions()
|
||||
.map((def) => `'${def.name}' (${def.description})`)
|
||||
.join(', ');
|
||||
|
||||
throw new Error(
|
||||
`Agent '${this.params.agent_name}' not found. Available agents are: ${availableAgents}. Please choose a valid agent_name.`,
|
||||
);
|
||||
}
|
||||
|
||||
const { agent_name: _agent_name, ...agentArgs } = this.params;
|
||||
|
||||
// Validate specific agent arguments here using SchemaValidator to generate helpful error messages.
|
||||
const validationError = SchemaValidator.validate(
|
||||
definition.inputConfig.inputSchema,
|
||||
agentArgs,
|
||||
);
|
||||
|
||||
if (validationError) {
|
||||
throw new Error(
|
||||
`Invalid arguments for agent '${definition.name}': ${validationError}. Input schema: ${JSON.stringify(definition.inputConfig.inputSchema)}.`,
|
||||
);
|
||||
}
|
||||
|
||||
const invocation = this.buildSubInvocation(
|
||||
definition,
|
||||
agentArgs as AgentInputs,
|
||||
);
|
||||
|
||||
return invocation.execute(signal, updateOutput);
|
||||
}
|
||||
|
||||
private buildSubInvocation(
|
||||
definition: AgentDefinition,
|
||||
agentArgs: AgentInputs,
|
||||
): ToolInvocation<AgentInputs, ToolResult> {
|
||||
const wrapper = new SubagentToolWrapper(
|
||||
definition,
|
||||
this.config,
|
||||
this.messageBus,
|
||||
);
|
||||
|
||||
return wrapper.build(agentArgs);
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,6 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { GeneralistAgent } from './generalist-agent.js';
|
||||
import { makeFakeConfig } from '../test-utils/config.js';
|
||||
import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js';
|
||||
import type { ToolRegistry } from '../tools/tool-registry.js';
|
||||
import type { AgentRegistry } from './registry.js';
|
||||
|
||||
@@ -15,10 +14,11 @@ describe('GeneralistAgent', () => {
|
||||
it('should create a valid generalist agent definition', () => {
|
||||
const config = makeFakeConfig();
|
||||
vi.spyOn(config, 'getToolRegistry').mockReturnValue({
|
||||
getAllToolNames: () => ['tool1', 'tool2', DELEGATE_TO_AGENT_TOOL_NAME],
|
||||
getAllToolNames: () => ['tool1', 'tool2', 'agent-tool'],
|
||||
} as unknown as ToolRegistry);
|
||||
vi.spyOn(config, 'getAgentRegistry').mockReturnValue({
|
||||
getDirectoryContext: () => 'mock directory context',
|
||||
getAllAgentNames: () => ['agent-tool'],
|
||||
} as unknown as AgentRegistry);
|
||||
|
||||
const agent = GeneralistAgent(config);
|
||||
@@ -27,7 +27,7 @@ describe('GeneralistAgent', () => {
|
||||
expect(agent.kind).toBe('local');
|
||||
expect(agent.modelConfig.model).toBe('inherit');
|
||||
expect(agent.toolConfig?.tools).toBeDefined();
|
||||
expect(agent.toolConfig?.tools).not.toContain(DELEGATE_TO_AGENT_TOOL_NAME);
|
||||
expect(agent.toolConfig?.tools).toContain('agent-tool');
|
||||
expect(agent.toolConfig?.tools).toContain('tool1');
|
||||
expect(agent.promptConfig.systemPrompt).toContain('CLI agent');
|
||||
// Ensure it's non-interactive
|
||||
|
||||
@@ -8,7 +8,6 @@ import { z } from 'zod';
|
||||
import type { Config } from '../config/config.js';
|
||||
import { getCoreSystemPrompt } from '../core/prompts.js';
|
||||
import type { LocalAgentDefinition } from './types.js';
|
||||
import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js';
|
||||
|
||||
const GeneralistAgentSchema = z.object({
|
||||
response: z.string().describe('The final response from the agent.'),
|
||||
@@ -48,11 +47,7 @@ export const GeneralistAgent = (
|
||||
model: 'inherit',
|
||||
},
|
||||
get toolConfig() {
|
||||
// TODO(15179): Support recursive agent invocation.
|
||||
const tools = config
|
||||
.getToolRegistry()
|
||||
.getAllToolNames()
|
||||
.filter((name) => name !== DELEGATE_TO_AGENT_TOOL_NAME);
|
||||
const tools = config.getToolRegistry().getAllToolNames();
|
||||
return {
|
||||
tools,
|
||||
};
|
||||
|
||||
@@ -61,6 +61,7 @@ import type {
|
||||
ModelConfigKey,
|
||||
ResolvedModelConfig,
|
||||
} from '../services/modelConfigService.js';
|
||||
import type { AgentRegistry } from './registry.js';
|
||||
import { getModelConfigAlias } from './registry.js';
|
||||
import type { ModelRouterService } from '../routing/modelRouterService.js';
|
||||
|
||||
@@ -298,6 +299,9 @@ describe('LocalAgentExecutor', () => {
|
||||
parentToolRegistry.registerTool(MOCK_TOOL_NOT_ALLOWED);
|
||||
|
||||
vi.spyOn(mockConfig, 'getToolRegistry').mockReturnValue(parentToolRegistry);
|
||||
vi.spyOn(mockConfig, 'getAgentRegistry').mockReturnValue({
|
||||
getAllAgentNames: () => [],
|
||||
} as unknown as AgentRegistry);
|
||||
|
||||
mockedGetDirectoryContextString.mockResolvedValue(
|
||||
'Mocked Environment Context',
|
||||
@@ -411,6 +415,32 @@ describe('LocalAgentExecutor', () => {
|
||||
const secondPart = startHistory?.[1]?.parts?.[0];
|
||||
expect(secondPart?.text).toBe('OK, starting on TestGoal.');
|
||||
});
|
||||
|
||||
it('should filter out subagent tools to prevent recursion', async () => {
|
||||
const subAgentName = 'recursive-agent';
|
||||
// Register a mock tool that simulates a subagent
|
||||
parentToolRegistry.registerTool(new MockTool({ name: subAgentName }));
|
||||
|
||||
// Mock the agent registry to return the subagent name
|
||||
vi.spyOn(
|
||||
mockConfig.getAgentRegistry(),
|
||||
'getAllAgentNames',
|
||||
).mockReturnValue([subAgentName]);
|
||||
|
||||
const definition = createTestDefinition([LS_TOOL_NAME, subAgentName]);
|
||||
const executor = await LocalAgentExecutor.create(
|
||||
definition,
|
||||
mockConfig,
|
||||
onActivity,
|
||||
);
|
||||
|
||||
const agentRegistry = executor['toolRegistry'];
|
||||
|
||||
// LS should be present
|
||||
expect(agentRegistry.getTool(LS_TOOL_NAME)).toBeDefined();
|
||||
// Subagent should be filtered out
|
||||
expect(agentRegistry.getTool(subAgentName)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('run (Execution Loop and Logic)', () => {
|
||||
|
||||
@@ -110,10 +110,22 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
|
||||
runtimeContext.getMessageBus(),
|
||||
);
|
||||
const parentToolRegistry = runtimeContext.getToolRegistry();
|
||||
const allAgentNames = new Set(
|
||||
runtimeContext.getAgentRegistry().getAllAgentNames(),
|
||||
);
|
||||
|
||||
if (definition.toolConfig) {
|
||||
for (const toolRef of definition.toolConfig.tools) {
|
||||
if (typeof toolRef === 'string') {
|
||||
// Check if the tool is a subagent to prevent recursion.
|
||||
// We do not allow agents to call other agents.
|
||||
if (allAgentNames.has(toolRef)) {
|
||||
debugLogger.warn(
|
||||
`[LocalAgentExecutor] Skipping subagent tool '${toolRef}' for agent '${definition.name}' to prevent recursion.`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// If the tool is referenced by name, retrieve it from the parent
|
||||
// registry and register it with the agent's isolated registry.
|
||||
const toolFromParent = parentToolRegistry.getTool(toolRef);
|
||||
|
||||
@@ -943,10 +943,10 @@ describe('AgentRegistry', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getToolDescription', () => {
|
||||
describe('getDirectoryContext', () => {
|
||||
it('should return default message when no agents are registered', () => {
|
||||
expect(registry.getToolDescription()).toContain(
|
||||
'No agents are currently available',
|
||||
expect(registry.getDirectoryContext()).toContain(
|
||||
'No sub-agents are currently available.',
|
||||
);
|
||||
});
|
||||
|
||||
@@ -958,18 +958,12 @@ describe('AgentRegistry', () => {
|
||||
description: 'Another agent description',
|
||||
});
|
||||
|
||||
const description = registry.getToolDescription();
|
||||
const description = registry.getDirectoryContext();
|
||||
|
||||
expect(description).toContain(
|
||||
'Delegates a task to a specialized sub-agent',
|
||||
);
|
||||
expect(description).toContain('Available agents:');
|
||||
expect(description).toContain(
|
||||
`- **${MOCK_AGENT_V1.name}**: ${MOCK_AGENT_V1.description}`,
|
||||
);
|
||||
expect(description).toContain(
|
||||
`- **AnotherAgent**: Another agent description`,
|
||||
);
|
||||
expect(description).toContain('Sub-agents are specialized expert agents');
|
||||
expect(description).toContain('Available Sub-Agents');
|
||||
expect(description).toContain(`- ${MOCK_AGENT_V1.name}`);
|
||||
expect(description).toContain(`- AnotherAgent`);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -21,7 +21,6 @@ import {
|
||||
type ModelConfig,
|
||||
ModelConfigService,
|
||||
} from '../services/modelConfigService.js';
|
||||
import { DELEGATE_TO_AGENT_TOOL_NAME } from '../tools/tool-names.js';
|
||||
|
||||
/**
|
||||
* Returns the model config alias for a given agent definition.
|
||||
@@ -393,23 +392,6 @@ export class AgentRegistry {
|
||||
return this.allDefinitions.get(name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Generates a description for the delegate_to_agent tool.
|
||||
* Unlike getDirectoryContext() which is for system prompts,
|
||||
* this is formatted for tool descriptions.
|
||||
*/
|
||||
getToolDescription(): string {
|
||||
if (this.agents.size === 0) {
|
||||
return 'Delegates a task to a specialized sub-agent. No agents are currently available.';
|
||||
}
|
||||
|
||||
const agentDescriptions = Array.from(this.agents.entries())
|
||||
.map(([name, def]) => `- **${name}**: ${def.description}`)
|
||||
.join('\n');
|
||||
|
||||
return `Delegates a task to a specialized sub-agent.\n\nAvailable agents:\n${agentDescriptions}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generates a markdown "Phone Book" of available agents and their schemas.
|
||||
* This MUST be injected into the System Prompt of the parent agent.
|
||||
@@ -423,20 +405,23 @@ export class AgentRegistry {
|
||||
context += `Sub-agents are specialized expert agents that you can use to assist you in
|
||||
the completion of all or part of a task.
|
||||
|
||||
ALWAYS use \`${DELEGATE_TO_AGENT_TOOL_NAME}\` to delegate to a subagent if one
|
||||
exists that has expertise relevant to your task.
|
||||
Each sub-agent is available as a tool of the same name.
|
||||
|
||||
For example:
|
||||
- Prompt: 'Fix test', Description: 'An agent with expertise in fixing tests.' -> should use the sub-agent.
|
||||
- Prompt: 'Update the license header', Description: 'An agent with expertise in licensing and copyright.' -> should use the sub-agent.
|
||||
- Prompt: 'Diagram the architecture of the codebase', Description: 'Agent with architecture experience'. -> should use the sub-agent.
|
||||
- Prompt: 'Implement a fix for [bug]' -> Should decompose the project into subtasks, which may utilize available agents like 'plan', 'validate', and 'fix-tests'.
|
||||
You MUST always delegate tasks to the sub-agent with the
|
||||
relevant expertise, if one is available.
|
||||
|
||||
The following are the available sub-agents:\n\n`;
|
||||
The following tools can be used to start sub-agents:\n\n`;
|
||||
|
||||
for (const [name, def] of this.agents) {
|
||||
context += `- **${name}**: ${def.description}\n`;
|
||||
for (const [name] of this.agents) {
|
||||
context += `- ${name}\n`;
|
||||
}
|
||||
|
||||
context += `Remember that the closest relevant sub-agent should still be used even if its expertise is broader than the given task.
|
||||
|
||||
For example:
|
||||
- A license-agent -> Should be used for a range of tasks, including reading, validating, and updating licenses and headers.
|
||||
- A test-fixing-agent -> Should be used both for fixing tests as well as investigating test failures.`;
|
||||
|
||||
return context;
|
||||
}
|
||||
}
|
||||
|
||||
132
packages/core/src/agents/subagent-tool.ts
Normal file
132
packages/core/src/agents/subagent-tool.ts
Normal file
@@ -0,0 +1,132 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import {
|
||||
BaseDeclarativeTool,
|
||||
Kind,
|
||||
type ToolInvocation,
|
||||
type ToolResult,
|
||||
BaseToolInvocation,
|
||||
type ToolCallConfirmationDetails,
|
||||
} from '../tools/tools.js';
|
||||
import type { AnsiOutput } from '../utils/terminalSerializer.js';
|
||||
import type { Config } from '../config/config.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import type { AgentDefinition, AgentInputs } from './types.js';
|
||||
import { SubagentToolWrapper } from './subagent-tool-wrapper.js';
|
||||
import { SchemaValidator } from '../utils/schemaValidator.js';
|
||||
|
||||
export class SubagentTool extends BaseDeclarativeTool<AgentInputs, ToolResult> {
|
||||
constructor(
|
||||
private readonly definition: AgentDefinition,
|
||||
private readonly config: Config,
|
||||
messageBus: MessageBus,
|
||||
) {
|
||||
const inputSchema = definition.inputConfig.inputSchema;
|
||||
|
||||
// Validate schema on construction
|
||||
const schemaError = SchemaValidator.validateSchema(inputSchema);
|
||||
if (schemaError) {
|
||||
throw new Error(
|
||||
`Invalid schema for agent ${definition.name}: ${schemaError}`,
|
||||
);
|
||||
}
|
||||
|
||||
super(
|
||||
definition.name,
|
||||
definition.displayName ?? definition.name,
|
||||
definition.description,
|
||||
Kind.Think,
|
||||
inputSchema,
|
||||
messageBus,
|
||||
/* isOutputMarkdown */ true,
|
||||
/* canUpdateOutput */ true,
|
||||
);
|
||||
}
|
||||
|
||||
protected createInvocation(
|
||||
params: AgentInputs,
|
||||
messageBus: MessageBus,
|
||||
_toolName?: string,
|
||||
_toolDisplayName?: string,
|
||||
): ToolInvocation<AgentInputs, ToolResult> {
|
||||
return new SubAgentInvocation(
|
||||
params,
|
||||
this.definition,
|
||||
this.config,
|
||||
messageBus,
|
||||
_toolName,
|
||||
_toolDisplayName,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
class SubAgentInvocation extends BaseToolInvocation<AgentInputs, ToolResult> {
|
||||
constructor(
|
||||
params: AgentInputs,
|
||||
private readonly definition: AgentDefinition,
|
||||
private readonly config: Config,
|
||||
messageBus: MessageBus,
|
||||
_toolName?: string,
|
||||
_toolDisplayName?: string,
|
||||
) {
|
||||
super(
|
||||
params,
|
||||
messageBus,
|
||||
_toolName ?? definition.name,
|
||||
_toolDisplayName ?? definition.displayName ?? definition.name,
|
||||
);
|
||||
}
|
||||
|
||||
getDescription(): string {
|
||||
return `Delegating to agent '${this.definition.name}'`;
|
||||
}
|
||||
|
||||
override async shouldConfirmExecute(
|
||||
abortSignal: AbortSignal,
|
||||
): Promise<ToolCallConfirmationDetails | false> {
|
||||
if (this.definition.kind !== 'remote') {
|
||||
// Local agents should execute without confirmation. Inner tool calls will bubble up their own confirmations to the user.
|
||||
return false;
|
||||
}
|
||||
|
||||
const invocation = this.buildSubInvocation(this.definition, this.params);
|
||||
return invocation.shouldConfirmExecute(abortSignal);
|
||||
}
|
||||
|
||||
async execute(
|
||||
signal: AbortSignal,
|
||||
updateOutput?: (output: string | AnsiOutput) => void,
|
||||
): Promise<ToolResult> {
|
||||
const validationError = SchemaValidator.validate(
|
||||
this.definition.inputConfig.inputSchema,
|
||||
this.params,
|
||||
);
|
||||
|
||||
if (validationError) {
|
||||
throw new Error(
|
||||
`Invalid arguments for agent '${this.definition.name}': ${validationError}. Input schema: ${JSON.stringify(this.definition.inputConfig.inputSchema)}.`,
|
||||
);
|
||||
}
|
||||
|
||||
const invocation = this.buildSubInvocation(this.definition, this.params);
|
||||
|
||||
return invocation.execute(signal, updateOutput);
|
||||
}
|
||||
|
||||
private buildSubInvocation(
|
||||
definition: AgentDefinition,
|
||||
agentArgs: AgentInputs,
|
||||
): ToolInvocation<AgentInputs, ToolResult> {
|
||||
const wrapper = new SubagentToolWrapper(
|
||||
definition,
|
||||
this.config,
|
||||
this.messageBus,
|
||||
);
|
||||
|
||||
return wrapper.build(agentArgs);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user