diff --git a/packages/core/src/agents/codebase-investigator.ts b/packages/core/src/agents/codebase-investigator.ts index 947fc1f9c4..0961535292 100644 --- a/packages/core/src/agents/codebase-investigator.ts +++ b/packages/core/src/agents/codebase-investigator.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { AgentDefinition } from './types.js'; +import type { LocalAgentDefinition } from './types.js'; import { GLOB_TOOL_NAME, GREP_TOOL_NAME, @@ -41,18 +41,19 @@ const CodebaseInvestigationReportSchema = z.object({ * A Proof-of-Concept subagent specialized in analyzing codebase structure, * dependencies, and technologies. */ -export const CodebaseInvestigatorAgent: AgentDefinition< +export const CodebaseInvestigatorAgent: LocalAgentDefinition< typeof CodebaseInvestigationReportSchema > = { name: 'codebase_investigator', + kind: 'local', displayName: 'Codebase Investigator Agent', - description: `The specialized tool for codebase analysis, architectural mapping, and understanding system-wide dependencies. - Invoke this tool for tasks like vague requests, bug root-cause analysis, system refactoring, comprehensive feature implementation or to answer questions about the codebase that require investigation. + description: `The specialized tool for codebase analysis, architectural mapping, and understanding system-wide dependencies. + Invoke this tool for tasks like vague requests, bug root-cause analysis, system refactoring, comprehensive feature implementation or to answer questions about the codebase that require investigation. It returns a structured report with key file paths, symbols, and actionable architectural insights.`, inputConfig: { inputs: { objective: { - description: `A comprehensive and detailed description of the user's ultimate goal. + description: `A comprehensive and detailed description of the user's ultimate goal. You must include original user's objective as well as questions and any extra context and questions you may have.`, type: 'string', required: true, 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 ebf753db30..009b6a0010 100644 --- a/packages/core/src/agents/delegate-to-agent-tool.test.ts +++ b/packages/core/src/agents/delegate-to-agent-tool.test.ts @@ -9,13 +9,13 @@ import { DelegateToAgentTool } 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 { SubagentInvocation } from './invocation.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'; -vi.mock('./invocation.js', () => ({ - SubagentInvocation: vi.fn().mockImplementation(() => ({ +vi.mock('./local-invocation.js', () => ({ + LocalSubagentInvocation: vi.fn().mockImplementation(() => ({ execute: vi .fn() .mockResolvedValue({ content: [{ type: 'text', text: 'Success' }] }), @@ -29,6 +29,7 @@ describe('DelegateToAgentTool', () => { let messageBus: MessageBus; const mockAgentDef: AgentDefinition = { + kind: 'local', name: 'test_agent', description: 'A test agent', promptConfig: {}, @@ -93,10 +94,10 @@ describe('DelegateToAgentTool', () => { const result = await invocation.execute(new AbortController().signal); expect(result).toEqual({ content: [{ type: 'text', text: 'Success' }] }); - expect(SubagentInvocation).toHaveBeenCalledWith( - { arg1: 'valid' }, + expect(LocalSubagentInvocation).toHaveBeenCalledWith( mockAgentDef, config, + { arg1: 'valid' }, messageBus, ); }); diff --git a/packages/core/src/agents/delegate-to-agent-tool.ts b/packages/core/src/agents/delegate-to-agent-tool.ts index 9993507e08..7fa42c80a5 100644 --- a/packages/core/src/agents/delegate-to-agent-tool.ts +++ b/packages/core/src/agents/delegate-to-agent-tool.ts @@ -18,7 +18,7 @@ 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 { SubagentInvocation } from './invocation.js'; +import { SubagentToolWrapper } from './subagent-tool-wrapper.js'; import type { AgentInputs } from './types.js'; type DelegateParams = { agent_name: string } & Record; @@ -169,14 +169,18 @@ class DelegateInvocation extends BaseToolInvocation< // eslint-disable-next-line @typescript-eslint/no-unused-vars const { agent_name, ...agentArgs } = this.params; - // Instantiate the Subagent Loop - const subagentInvocation = new SubagentInvocation( - agentArgs as AgentInputs, + // Delegate the creation of the specific invocation (Local or Remote) to the wrapper. + // This centralizes the logic and ensures consistent handling. + const wrapper = new SubagentToolWrapper( definition, this.config, this.messageBus, ); - return subagentInvocation.execute(signal, updateOutput); + // We could skip extra validation here if we trust the Registry's schema, + // but build() will do a safety check anyway. + const invocation = wrapper.build(agentArgs as AgentInputs); + + return invocation.execute(signal, updateOutput); } } diff --git a/packages/core/src/agents/executor.test.ts b/packages/core/src/agents/local-executor.test.ts similarity index 95% rename from packages/core/src/agents/executor.test.ts rename to packages/core/src/agents/local-executor.test.ts index 230fec702e..a1e635df4f 100644 --- a/packages/core/src/agents/executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -14,7 +14,7 @@ import { type Mock, } from 'vitest'; import { debugLogger } from '../utils/debugLogger.js'; -import { AgentExecutor, type ActivityCallback } from './executor.js'; +import { LocalAgentExecutor, type ActivityCallback } from './local-executor.js'; import { makeFakeConfig } from '../test-utils/config.js'; import { ToolRegistry } from '../tools/tool-registry.js'; import { LSTool } from '../tools/ls.js'; @@ -48,8 +48,8 @@ import { RecoveryAttemptEvent, } from '../telemetry/types.js'; import type { - AgentDefinition, AgentInputs, + LocalAgentDefinition, SubagentActivityEvent, OutputConfig, } from './types.js'; @@ -193,12 +193,15 @@ let parentToolRegistry: ToolRegistry; /** * Type-safe helper to create agent definitions for tests. */ -const createTestDefinition = ( + +export const createTestDefinition = < + TOutput extends z.ZodTypeAny = z.ZodUnknown, +>( tools: Array = [LS_TOOL_NAME], - runConfigOverrides: Partial['runConfig']> = {}, + runConfigOverrides: Partial['runConfig']> = {}, outputConfigMode: 'default' | 'none' = 'default', schema: TOutput = z.string() as unknown as TOutput, -): AgentDefinition => { +): LocalAgentDefinition => { let outputConfig: OutputConfig | undefined; if (outputConfigMode === 'default') { @@ -210,6 +213,7 @@ const createTestDefinition = ( } return { + kind: 'local', name: 'TestAgent', description: 'An agent for testing.', inputConfig: { @@ -223,7 +227,7 @@ const createTestDefinition = ( }; }; -describe('AgentExecutor', () => { +describe('LocalAgentExecutor', () => { let activities: SubagentActivityEvent[]; let onActivity: ActivityCallback; let abortController: AbortController; @@ -289,18 +293,18 @@ describe('AgentExecutor', () => { describe('create (Initialization and Validation)', () => { it('should create successfully with allowed tools', async () => { const definition = createTestDefinition([LS_TOOL_NAME]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, ); - expect(executor).toBeInstanceOf(AgentExecutor); + expect(executor).toBeInstanceOf(LocalAgentExecutor); }); it('SECURITY: should throw if a tool is not on the non-interactive allowlist', async () => { const definition = createTestDefinition([MOCK_TOOL_NOT_ALLOWED.name]); await expect( - AgentExecutor.create(definition, mockConfig, onActivity), + LocalAgentExecutor.create(definition, mockConfig, onActivity), ).rejects.toThrow(/not on the allow-list for non-interactive execution/); }); @@ -309,7 +313,7 @@ describe('AgentExecutor', () => { LS_TOOL_NAME, READ_FILE_TOOL_NAME, ]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -330,7 +334,7 @@ describe('AgentExecutor', () => { mockedPromptIdContext.getStore.mockReturnValue(parentId); const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -361,7 +365,7 @@ describe('AgentExecutor', () => { }, ]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -390,7 +394,7 @@ describe('AgentExecutor', () => { definition.inputConfig.inputs = { goal: { type: 'string', required: true, description: 'goal' }, }; - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -413,7 +417,7 @@ describe('AgentExecutor', () => { it('should execute successfully when model calls complete_task with output (Happy Path with Output)', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -562,7 +566,7 @@ describe('AgentExecutor', () => { it('should execute successfully when model calls complete_task without output (Happy Path No Output)', async () => { const definition = createTestDefinition([LS_TOOL_NAME], {}, 'none'); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -629,7 +633,7 @@ describe('AgentExecutor', () => { it('should error immediately if the model stops tools without calling complete_task (Protocol Violation)', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -705,7 +709,7 @@ describe('AgentExecutor', () => { it('should report an error if complete_task is called with missing required arguments', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -768,7 +772,7 @@ describe('AgentExecutor', () => { it('should handle multiple calls to complete_task in the same turn (accept first, block rest)', async () => { const definition = createTestDefinition([], {}, 'none'); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -803,7 +807,7 @@ describe('AgentExecutor', () => { it('should execute parallel tool calls and then complete', async () => { const definition = createTestDefinition([LS_TOOL_NAME]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -898,7 +902,7 @@ describe('AgentExecutor', () => { it('SECURITY: should block unauthorized tools and provide explicit failure to model', async () => { const definition = createTestDefinition([LS_TOOL_NAME]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -934,7 +938,7 @@ describe('AgentExecutor', () => { // 2. Verify console warning expect(consoleWarnSpy).toHaveBeenCalledWith( - expect.stringContaining(`[AgentExecutor] Blocked call:`), + expect.stringContaining(`[LocalAgentExecutor] Blocked call:`), ); consoleWarnSpy.mockRestore(); @@ -975,7 +979,7 @@ describe('AgentExecutor', () => { 'default', z.string().min(10), // The schema is for the output value itself ); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1044,7 +1048,7 @@ describe('AgentExecutor', () => { }); // We expect the error to be thrown during the run, not creation - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1075,7 +1079,7 @@ describe('AgentExecutor', () => { it('should handle a failed tool call and feed the error to the model', async () => { const definition = createTestDefinition([LS_TOOL_NAME]); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1196,7 +1200,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_turns: MAX, }); - const executor = await AgentExecutor.create(definition, mockConfig); + const executor = await LocalAgentExecutor.create(definition, mockConfig); mockWorkResponse('t1'); mockWorkResponse('t2'); @@ -1213,7 +1217,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_time_minutes: 0.5, // 30 seconds }); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1270,7 +1274,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_time_minutes: 1, }); - const executor = await AgentExecutor.create(definition, mockConfig); + const executor = await LocalAgentExecutor.create(definition, mockConfig); mockModelResponse([ { name: LS_TOOL_NAME, args: { path: '.' }, id: 't1' }, @@ -1306,7 +1310,7 @@ describe('AgentExecutor', () => { it('should terminate when AbortSignal is triggered', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create(definition, mockConfig); + const executor = await LocalAgentExecutor.create(definition, mockConfig); mockSendMessageStream.mockImplementationOnce(async () => (async function* () { @@ -1358,7 +1362,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_turns: MAX, }); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1406,7 +1410,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_turns: MAX, }); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1440,7 +1444,7 @@ describe('AgentExecutor', () => { it('should recover successfully from a protocol violation (no complete_task)', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1482,7 +1486,7 @@ describe('AgentExecutor', () => { it('should fail recovery from a protocol violation if it violates again', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1525,7 +1529,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_time_minutes: 0.5, // 30 seconds }); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1580,7 +1584,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_time_minutes: 0.5, // 30 seconds }); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1672,7 +1676,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_turns: MAX, }); - const executor = await AgentExecutor.create(definition, mockConfig); + const executor = await LocalAgentExecutor.create(definition, mockConfig); // Turn 1 (hits max_turns) mockWorkResponse('t1'); @@ -1697,7 +1701,7 @@ describe('AgentExecutor', () => { const definition = createTestDefinition([LS_TOOL_NAME], { max_turns: MAX, }); - const executor = await AgentExecutor.create(definition, mockConfig); + const executor = await LocalAgentExecutor.create(definition, mockConfig); // Turn 1 (hits max_turns) mockWorkResponse('t1'); @@ -1752,7 +1756,7 @@ describe('AgentExecutor', () => { it('should attempt to compress chat history on each turn', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1786,7 +1790,7 @@ describe('AgentExecutor', () => { it('should update chat history when compression is successful', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1821,7 +1825,7 @@ describe('AgentExecutor', () => { it('should pass hasFailedCompressionAttempt=true to compression after a failure', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, @@ -1866,7 +1870,7 @@ describe('AgentExecutor', () => { it('should reset hasFailedCompressionAttempt flag after a successful compression', async () => { const definition = createTestDefinition(); - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( definition, mockConfig, onActivity, diff --git a/packages/core/src/agents/executor.ts b/packages/core/src/agents/local-executor.ts similarity index 98% rename from packages/core/src/agents/executor.ts rename to packages/core/src/agents/local-executor.ts index a36c51e93f..52be227bc4 100644 --- a/packages/core/src/agents/executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -41,7 +41,7 @@ import { RecoveryAttemptEvent, } from '../telemetry/types.js'; import type { - AgentDefinition, + LocalAgentDefinition, AgentInputs, OutputObject, SubagentActivityEvent, @@ -78,8 +78,8 @@ type AgentTurnResult = * This executor runs the agent in a loop, calling tools until it calls the * mandatory `complete_task` tool to signal completion. */ -export class AgentExecutor { - readonly definition: AgentDefinition; +export class LocalAgentExecutor { + readonly definition: LocalAgentDefinition; private readonly agentId: string; private readonly toolRegistry: ToolRegistry; @@ -97,13 +97,13 @@ export class AgentExecutor { * @param definition The definition object for the agent. * @param runtimeContext The global runtime configuration. * @param onActivity An optional callback to receive activity events. - * @returns A promise that resolves to a new `AgentExecutor` instance. + * @returns A promise that resolves to a new `LocalAgentExecutor` instance. */ static async create( - definition: AgentDefinition, + definition: LocalAgentDefinition, runtimeContext: Config, onActivity?: ActivityCallback, - ): Promise> { + ): Promise> { // Create an isolated tool registry for this agent instance. const agentToolRegistry = new ToolRegistry(runtimeContext); const parentToolRegistry = runtimeContext.getToolRegistry(); @@ -131,13 +131,16 @@ export class AgentExecutor { agentToolRegistry.sortTools(); // Validate that all registered tools are safe for non-interactive // execution. - await AgentExecutor.validateTools(agentToolRegistry, definition.name); + await LocalAgentExecutor.validateTools( + agentToolRegistry, + definition.name, + ); } // Get the parent prompt ID from context const parentPromptId = promptIdContext.getStore(); - return new AgentExecutor( + return new LocalAgentExecutor( definition, runtimeContext, agentToolRegistry, @@ -153,7 +156,7 @@ export class AgentExecutor { * instantiate the class. */ private constructor( - definition: AgentDefinition, + definition: LocalAgentDefinition, runtimeContext: Config, toolRegistry: ToolRegistry, parentPromptId: string | undefined, @@ -820,7 +823,7 @@ export class AgentExecutor { if (!allowedToolNames.has(functionCall.name as string)) { const error = `Unauthorized tool call: '${functionCall.name}' is not available to this agent.`; - debugLogger.warn(`[AgentExecutor] Blocked call: ${error}`); + debugLogger.warn(`[LocalAgentExecutor] Blocked call: ${error}`); syncResponseParts.push({ functionResponse: { diff --git a/packages/core/src/agents/invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts similarity index 86% rename from packages/core/src/agents/invocation.test.ts rename to packages/core/src/agents/local-invocation.test.ts index a91f5ee80e..3aa5a39628 100644 --- a/packages/core/src/agents/invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -5,13 +5,10 @@ */ import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest'; -import { SubagentInvocation } from './invocation.js'; -import { AgentExecutor } from './executor.js'; -import type { - AgentDefinition, - SubagentActivityEvent, - AgentInputs, -} from './types.js'; +import type { LocalAgentDefinition } from './types.js'; +import { LocalSubagentInvocation } from './local-invocation.js'; +import { LocalAgentExecutor } from './local-executor.js'; +import type { SubagentActivityEvent, AgentInputs } from './types.js'; import { AgentTerminateMode } from './types.js'; import { makeFakeConfig } from '../test-utils/config.js'; import { ToolErrorType } from '../tools/tool-error.js'; @@ -19,13 +16,14 @@ import type { Config } from '../config/config.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { type z } from 'zod'; -vi.mock('./executor.js'); +vi.mock('./local-executor.js'); -const MockAgentExecutor = vi.mocked(AgentExecutor); +const MockLocalAgentExecutor = vi.mocked(LocalAgentExecutor); let mockConfig: Config; -const testDefinition: AgentDefinition = { +const testDefinition: LocalAgentDefinition = { + kind: 'local', name: 'MockAgent', description: 'A mock agent.', inputConfig: { @@ -39,8 +37,8 @@ const testDefinition: AgentDefinition = { promptConfig: { systemPrompt: 'test' }, }; -describe('SubagentInvocation', () => { - let mockExecutorInstance: Mocked>; +describe('LocalSubagentInvocation', () => { + let mockExecutorInstance: Mocked>; beforeEach(() => { vi.clearAllMocks(); @@ -49,20 +47,20 @@ describe('SubagentInvocation', () => { mockExecutorInstance = { run: vi.fn(), definition: testDefinition, - } as unknown as Mocked>; + } as unknown as Mocked>; - MockAgentExecutor.create.mockResolvedValue( - mockExecutorInstance as unknown as AgentExecutor, + MockLocalAgentExecutor.create.mockResolvedValue( + mockExecutorInstance as unknown as LocalAgentExecutor, ); }); it('should pass the messageBus to the parent constructor', () => { const mockMessageBus = {} as MessageBus; const params = { task: 'Analyze data' }; - const invocation = new SubagentInvocation( - params, + const invocation = new LocalSubagentInvocation( testDefinition, mockConfig, + params, mockMessageBus, ); @@ -74,10 +72,10 @@ describe('SubagentInvocation', () => { describe('getDescription', () => { it('should format the description with inputs', () => { const params = { task: 'Analyze data', priority: 5 }; - const invocation = new SubagentInvocation( - params, + const invocation = new LocalSubagentInvocation( testDefinition, mockConfig, + params, ); const description = invocation.getDescription(); expect(description).toBe( @@ -88,10 +86,10 @@ describe('SubagentInvocation', () => { it('should truncate long input values', () => { const longTask = 'A'.repeat(100); const params = { task: longTask }; - const invocation = new SubagentInvocation( - params, + const invocation = new LocalSubagentInvocation( testDefinition, mockConfig, + params, ); const description = invocation.getDescription(); // Default INPUT_PREVIEW_MAX_LENGTH is 50 @@ -102,7 +100,7 @@ describe('SubagentInvocation', () => { it('should truncate the overall description if it exceeds the limit', () => { // Create a definition and inputs that result in a very long description - const longNameDef = { + const longNameDef: LocalAgentDefinition = { ...testDefinition, name: 'VeryLongAgentNameThatTakesUpSpace', }; @@ -110,10 +108,10 @@ describe('SubagentInvocation', () => { for (let i = 0; i < 20; i++) { params[`input${i}`] = `value${i}`; } - const invocation = new SubagentInvocation( - params, + const invocation = new LocalSubagentInvocation( longNameDef, mockConfig, + params, ); const description = invocation.getDescription(); // Default DESCRIPTION_MAX_LENGTH is 200 @@ -130,15 +128,15 @@ describe('SubagentInvocation', () => { let signal: AbortSignal; let updateOutput: ReturnType; const params = { task: 'Execute task' }; - let invocation: SubagentInvocation; + let invocation: LocalSubagentInvocation; beforeEach(() => { signal = new AbortController().signal; updateOutput = vi.fn(); - invocation = new SubagentInvocation( - params, + invocation = new LocalSubagentInvocation( testDefinition, mockConfig, + params, ); }); @@ -151,7 +149,7 @@ describe('SubagentInvocation', () => { const result = await invocation.execute(signal, updateOutput); - expect(MockAgentExecutor.create).toHaveBeenCalledWith( + expect(MockLocalAgentExecutor.create).toHaveBeenCalledWith( testDefinition, mockConfig, expect.any(Function), @@ -173,7 +171,7 @@ describe('SubagentInvocation', () => { it('should stream THOUGHT_CHUNK activities from the executor', async () => { mockExecutorInstance.run.mockImplementation(async () => { - const onActivity = MockAgentExecutor.create.mock.calls[0][2]; + const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; if (onActivity) { onActivity({ @@ -202,7 +200,7 @@ describe('SubagentInvocation', () => { it('should NOT stream other activities (e.g., TOOL_CALL_START, ERROR)', async () => { mockExecutorInstance.run.mockImplementation(async () => { - const onActivity = MockAgentExecutor.create.mock.calls[0][2]; + const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; if (onActivity) { onActivity({ @@ -230,7 +228,7 @@ describe('SubagentInvocation', () => { it('should run successfully without an updateOutput callback', async () => { mockExecutorInstance.run.mockImplementation(async () => { - const onActivity = MockAgentExecutor.create.mock.calls[0][2]; + const onActivity = MockLocalAgentExecutor.create.mock.calls[0][2]; if (onActivity) { // Ensure calling activity doesn't crash when updateOutput is undefined onActivity({ @@ -269,7 +267,7 @@ describe('SubagentInvocation', () => { it('should handle executor creation failure', async () => { const creationError = new Error('Failed to initialize tools.'); - MockAgentExecutor.create.mockRejectedValue(creationError); + MockLocalAgentExecutor.create.mockRejectedValue(creationError); const result = await invocation.execute(signal, updateOutput); diff --git a/packages/core/src/agents/invocation.ts b/packages/core/src/agents/local-invocation.ts similarity index 90% rename from packages/core/src/agents/invocation.ts rename to packages/core/src/agents/local-invocation.ts index cb49095cb5..ad27a85a61 100644 --- a/packages/core/src/agents/invocation.ts +++ b/packages/core/src/agents/local-invocation.ts @@ -5,17 +5,16 @@ */ import type { Config } from '../config/config.js'; -import { AgentExecutor } from './executor.js'; +import { LocalAgentExecutor } from './local-executor.js'; import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { BaseToolInvocation, type ToolResult } from '../tools/tools.js'; import { ToolErrorType } from '../tools/tool-error.js'; import type { - AgentDefinition, + LocalAgentDefinition, AgentInputs, SubagentActivityEvent, } from './types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; -import { type z } from 'zod'; const INPUT_PREVIEW_MAX_LENGTH = 50; const DESCRIPTION_MAX_LENGTH = 200; @@ -24,28 +23,29 @@ const DESCRIPTION_MAX_LENGTH = 200; * Represents a validated, executable instance of a subagent tool. * * This class orchestrates the execution of a defined agent by: - * 1. Initializing the {@link AgentExecutor}. + * 1. Initializing the {@link LocalAgentExecutor}. * 2. Running the agent's execution loop. * 3. Bridging the agent's streaming activity (e.g., thoughts) to the tool's * live output stream. * 4. Formatting the final result into a {@link ToolResult}. */ -export class SubagentInvocation< - TOutput extends z.ZodTypeAny, -> extends BaseToolInvocation { +export class LocalSubagentInvocation extends BaseToolInvocation< + AgentInputs, + ToolResult +> { /** - * @param params The validated input parameters for the agent. * @param definition The definition object that configures the agent. * @param config The global runtime configuration. + * @param params The validated input parameters for the agent. * @param messageBus Optional message bus for policy enforcement. */ constructor( - params: AgentInputs, - private readonly definition: AgentDefinition, + private readonly definition: LocalAgentDefinition, private readonly config: Config, + params: AgentInputs, messageBus?: MessageBus, ) { - super(params, messageBus); + super(params, messageBus, definition.name, definition.displayName); } /** @@ -94,7 +94,7 @@ export class SubagentInvocation< } }; - const executor = await AgentExecutor.create( + const executor = await LocalAgentExecutor.create( this.definition, this.config, onActivity, diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 8060815ac3..43ce617d9d 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { AgentRegistry, getModelConfigAlias } from './registry.js'; import { makeFakeConfig } from '../test-utils/config.js'; -import type { AgentDefinition } from './types.js'; +import type { AgentDefinition, LocalAgentDefinition } from './types.js'; import type { Config } from '../config/config.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -20,6 +20,7 @@ class TestableAgentRegistry extends AgentRegistry { // Define mock agent structures for testing registration logic const MOCK_AGENT_V1: AgentDefinition = { + kind: 'local', name: 'MockAgent', description: 'Mock Description V1', inputConfig: { inputs: {} }, @@ -89,7 +90,9 @@ describe('AgentRegistry', () => { 'codebase_investigator', ); expect(investigatorDef).toBeDefined(); - expect(investigatorDef?.modelConfig.model).toBe('gemini-3-pro-preview'); + expect((investigatorDef as LocalAgentDefinition).modelConfig.model).toBe( + 'gemini-3-pro-preview', + ); }); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 927b14c8cc..eff09fac86 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -115,26 +115,31 @@ export class AgentRegistry { // Register model config. // TODO(12916): Migrate sub-agents where possible to static configs. - const modelConfig = definition.modelConfig; + if (definition.kind === 'local') { + const modelConfig = definition.modelConfig; - const runtimeAlias: ModelConfigAlias = { - modelConfig: { - model: modelConfig.model, - generateContentConfig: { - temperature: modelConfig.temp, - topP: modelConfig.top_p, - thinkingConfig: { - includeThoughts: true, - thinkingBudget: modelConfig.thinkingBudget ?? -1, + const runtimeAlias: ModelConfigAlias = { + modelConfig: { + model: modelConfig.model, + generateContentConfig: { + temperature: modelConfig.temp, + topP: modelConfig.top_p, + thinkingConfig: { + includeThoughts: true, + thinkingBudget: modelConfig.thinkingBudget ?? -1, + }, }, }, - }, - }; + }; - this.config.modelConfigService.registerRuntimeModelConfig( - getModelConfigAlias(definition), - runtimeAlias, - ); + this.config.modelConfigService.registerRuntimeModelConfig( + getModelConfigAlias(definition), + runtimeAlias, + ); + } + + // Register configured remote A2A agents. + // TODO: Implement remote agent registration. } /** @@ -191,7 +196,7 @@ ${agentDescriptions}`; context += 'Use `delegate_to_agent` for complex tasks requiring specialized analysis.\n\n'; - for (const [name, def] of this.agents.entries()) { + for (const [name, def] of this.agents) { context += `- **${name}**: ${def.description}\n`; } return context; diff --git a/packages/core/src/agents/remote-invocation.test.ts b/packages/core/src/agents/remote-invocation.test.ts new file mode 100644 index 0000000000..bbe6d15f31 --- /dev/null +++ b/packages/core/src/agents/remote-invocation.test.ts @@ -0,0 +1,47 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import type { ToolCallConfirmationDetails } from '../tools/tools.js'; +import { RemoteAgentInvocation } from './remote-invocation.js'; +import type { RemoteAgentDefinition } from './types.js'; + +class TestableRemoteAgentInvocation extends RemoteAgentInvocation { + override async getConfirmationDetails( + abortSignal: AbortSignal, + ): Promise { + return super.getConfirmationDetails(abortSignal); + } +} + +describe('RemoteAgentInvocation', () => { + const mockDefinition: RemoteAgentDefinition = { + kind: 'remote', + name: 'test-remote-agent', + description: 'A test remote agent', + displayName: 'Test Remote Agent', + agentCardUrl: 'https://example.com/agent-card', + inputConfig: { + inputs: {}, + }, + }; + + it('should be instantiated with correct params', () => { + const invocation = new RemoteAgentInvocation(mockDefinition, {}); + expect(invocation).toBeDefined(); + expect(invocation.getDescription()).toBe( + 'Calling remote agent Test Remote Agent', + ); + }); + + it('should return false for confirmation details (not yet implemented)', async () => { + const invocation = new TestableRemoteAgentInvocation(mockDefinition, {}); + const details = await invocation.getConfirmationDetails( + new AbortController().signal, + ); + expect(details).toBe(false); + }); +}); diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts new file mode 100644 index 0000000000..ee52f2f388 --- /dev/null +++ b/packages/core/src/agents/remote-invocation.ts @@ -0,0 +1,48 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + BaseToolInvocation, + type ToolResult, + type ToolCallConfirmationDetails, +} from '../tools/tools.js'; +import type { AgentInputs, RemoteAgentDefinition } from './types.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; + +/** + * A tool invocation that proxies to a remote A2A agent. + * + * This implementation bypasses the local `LocalAgentExecutor` loop and directly + * invokes the configured A2A tool. + */ +export class RemoteAgentInvocation extends BaseToolInvocation< + AgentInputs, + ToolResult +> { + constructor( + private readonly definition: RemoteAgentDefinition, + params: AgentInputs, + messageBus?: MessageBus, + ) { + super(params, messageBus, definition.name, definition.displayName); + } + + getDescription(): string { + return `Calling remote agent ${this.definition.displayName ?? this.definition.name}`; + } + + protected override async getConfirmationDetails( + _abortSignal: AbortSignal, + ): Promise { + // TODO: Implement confirmation logic for remote agents. + return false; + } + + async execute(_signal: AbortSignal): Promise { + // TODO: Implement remote agent invocation logic. + throw new Error(`Remote agent invocation not implemented.`); + } +} diff --git a/packages/core/src/agents/subagent-tool-wrapper.test.ts b/packages/core/src/agents/subagent-tool-wrapper.test.ts index f971dc5162..382aafebf8 100644 --- a/packages/core/src/agents/subagent-tool-wrapper.test.ts +++ b/packages/core/src/agents/subagent-tool-wrapper.test.ts @@ -6,19 +6,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { SubagentToolWrapper } from './subagent-tool-wrapper.js'; -import { SubagentInvocation } from './invocation.js'; +import { LocalSubagentInvocation } from './local-invocation.js'; import { convertInputConfigToJsonSchema } from './schema-utils.js'; import { makeFakeConfig } from '../test-utils/config.js'; -import type { AgentDefinition, AgentInputs } from './types.js'; +import type { LocalAgentDefinition, AgentInputs } from './types.js'; import type { Config } from '../config/config.js'; import { Kind } from '../tools/tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; // Mock dependencies to isolate the SubagentToolWrapper class -vi.mock('./invocation.js'); +vi.mock('./local-invocation.js'); vi.mock('./schema-utils.js'); -const MockedSubagentInvocation = vi.mocked(SubagentInvocation); +const MockedLocalSubagentInvocation = vi.mocked(LocalSubagentInvocation); const mockConvertInputConfigToJsonSchema = vi.mocked( convertInputConfigToJsonSchema, ); @@ -26,7 +26,8 @@ const mockConvertInputConfigToJsonSchema = vi.mocked( // Define reusable test data let mockConfig: Config; -const mockDefinition: AgentDefinition = { +const mockDefinition: LocalAgentDefinition = { + kind: 'local', name: 'TestAgent', displayName: 'Test Agent Display Name', description: 'An agent for testing.', @@ -106,23 +107,23 @@ describe('SubagentToolWrapper', () => { }); describe('createInvocation', () => { - it('should create a SubagentInvocation with the correct parameters', () => { + it('should create a LocalSubagentInvocation with the correct parameters', () => { const wrapper = new SubagentToolWrapper(mockDefinition, mockConfig); const params: AgentInputs = { goal: 'Test the invocation', priority: 1 }; // The public `build` method calls the protected `createInvocation` after validation const invocation = wrapper.build(params); - expect(invocation).toBeInstanceOf(SubagentInvocation); - expect(MockedSubagentInvocation).toHaveBeenCalledExactlyOnceWith( - params, + expect(invocation).toBeInstanceOf(LocalSubagentInvocation); + expect(MockedLocalSubagentInvocation).toHaveBeenCalledExactlyOnceWith( mockDefinition, mockConfig, + params, undefined, ); }); - it('should pass the messageBus to the SubagentInvocation constructor', () => { + it('should pass the messageBus to the LocalSubagentInvocation constructor', () => { const mockMessageBus = {} as MessageBus; const wrapper = new SubagentToolWrapper( mockDefinition, @@ -133,10 +134,10 @@ describe('SubagentToolWrapper', () => { wrapper.build(params); - expect(MockedSubagentInvocation).toHaveBeenCalledWith( - params, + expect(MockedLocalSubagentInvocation).toHaveBeenCalledWith( mockDefinition, mockConfig, + params, mockMessageBus, ); }); @@ -151,7 +152,7 @@ describe('SubagentToolWrapper', () => { expect(() => wrapper.build(invalidParams)).toThrow( "params must have required property 'goal'", ); - expect(MockedSubagentInvocation).not.toHaveBeenCalled(); + expect(MockedLocalSubagentInvocation).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/core/src/agents/subagent-tool-wrapper.ts b/packages/core/src/agents/subagent-tool-wrapper.ts index ace4ee3478..69c96014cd 100644 --- a/packages/core/src/agents/subagent-tool-wrapper.ts +++ b/packages/core/src/agents/subagent-tool-wrapper.ts @@ -13,7 +13,8 @@ import { import type { Config } from '../config/config.js'; import type { AgentDefinition, AgentInputs } from './types.js'; import { convertInputConfigToJsonSchema } from './schema-utils.js'; -import { SubagentInvocation } from './invocation.js'; +import { LocalSubagentInvocation } from './local-invocation.js'; +import { RemoteAgentInvocation } from './remote-invocation.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; /** @@ -39,7 +40,6 @@ export class SubagentToolWrapper extends BaseDeclarativeTool< private readonly config: Config, messageBus?: MessageBus, ) { - // Dynamically generate the JSON schema required for the tool definition. const parameterSchema = convertInputConfigToJsonSchema( definition.inputConfig, ); @@ -68,10 +68,15 @@ export class SubagentToolWrapper extends BaseDeclarativeTool< protected createInvocation( params: AgentInputs, ): ToolInvocation { - return new SubagentInvocation( - params, - this.definition, + const definition = this.definition; + if (definition.kind === 'remote') { + return new RemoteAgentInvocation(definition, params, this.messageBus); + } + + return new LocalSubagentInvocation( + definition, this.config, + params, this.messageBus, ); } diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index 5a152ba8de..c42a3103ac 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -49,20 +49,33 @@ export interface SubagentActivityEvent { } /** - * The definition for an agent. + * The base definition for an agent. * @template TOutput The specific Zod schema for the agent's final output object. */ -export interface AgentDefinition { +export interface BaseAgentDefinition< + TOutput extends z.ZodTypeAny = z.ZodUnknown, +> { /** Unique identifier for the agent. */ name: string; displayName?: string; description: string; + inputConfig: InputConfig; + outputConfig?: OutputConfig; +} + +export interface LocalAgentDefinition< + TOutput extends z.ZodTypeAny = z.ZodUnknown, +> extends BaseAgentDefinition { + kind: 'local'; + + // Local agent required configs promptConfig: PromptConfig; modelConfig: ModelConfig; runConfig: RunConfig; + + // Optional configs toolConfig?: ToolConfig; - outputConfig?: OutputConfig; - inputConfig: InputConfig; + /** * An optional function to process the raw output from the agent's final tool * call into a string format. @@ -73,6 +86,17 @@ export interface AgentDefinition { processOutput?: (output: z.infer) => string; } +export interface RemoteAgentDefinition< + TOutput extends z.ZodTypeAny = z.ZodUnknown, +> extends BaseAgentDefinition { + kind: 'remote'; + agentCardUrl: string; +} + +export type AgentDefinition = + | LocalAgentDefinition + | RemoteAgentDefinition; + /** * Configures the initial prompt for the agent. */ diff --git a/packages/core/src/services/modelConfig.integration.test.ts b/packages/core/src/services/modelConfig.integration.test.ts index 7706f33078..c6daf962b6 100644 --- a/packages/core/src/services/modelConfig.integration.test.ts +++ b/packages/core/src/services/modelConfig.integration.test.ts @@ -236,7 +236,7 @@ describe('ModelConfigService Integration', () => { // Re-instantiate service for this isolated test to not pollute other tests const service = new ModelConfigService(complexConfig); - // Register a runtime alias, simulating what AgentExecutor does. + // Register a runtime alias, simulating what LocalAgentExecutor does. // This alias extends a static base and provides its own settings. service.registerRuntimeModelConfig('agent-runtime:my-agent', { extends: 'creative-writer', // extends a multi-level alias