diff --git a/packages/core/src/agents/a2a-client-manager.test.ts b/packages/core/src/agents/a2a-client-manager.test.ts index fb0f2829a4..4406cac966 100644 --- a/packages/core/src/agents/a2a-client-manager.test.ts +++ b/packages/core/src/agents/a2a-client-manager.test.ts @@ -340,5 +340,53 @@ describe('A2AClientManager', () => { expect(data.status.state).toBe('working'); }); + + it('bypasses adapter for JSON-RPC requests', async () => { + const baseFetch = vi.fn().mockResolvedValue(new Response('{}')); + const adapter = createAdapterFetch(baseFetch as typeof fetch); + const rpcBody = JSON.stringify({ jsonrpc: '2.0', method: 'foo' }); + + await adapter('http://example.com', { + method: 'POST', + body: rpcBody, + }); + + // Verify baseFetch was called with original body, not modified + expect(baseFetch).toHaveBeenCalledWith( + 'http://example.com', + expect.objectContaining({ body: rpcBody }), + ); + }); + + it('applies dialect translation for remote REST requests', async () => { + const baseFetch = vi.fn().mockResolvedValue(new Response('{}')); + const adapter = createAdapterFetch(baseFetch as typeof fetch); + const originalBody = JSON.stringify({ + message: { + role: 'user', + parts: [{ kind: 'text', text: 'hi' }], + }, + }); + + await adapter('https://remote-agent.com/v1/message:send', { + method: 'POST', + body: originalBody, + }); + + // Verify body WAS modified: + // 1. role: 'user' -> 'ROLE_USER' + // 2. parts mapped to content, kind stripped + const expectedBody = JSON.stringify({ + message: { + role: 'ROLE_USER', + content: [{ text: 'hi' }], + }, + }); + + expect(baseFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ body: expectedBody }), + ); + }); }); }); diff --git a/packages/core/src/agents/a2a-client-manager.ts b/packages/core/src/agents/a2a-client-manager.ts index ef93522e03..c00fdfee43 100644 --- a/packages/core/src/agents/a2a-client-manager.ts +++ b/packages/core/src/agents/a2a-client-manager.ts @@ -232,25 +232,24 @@ export function createAdapterFetch(baseFetch: typeof fetch): typeof fetch { input: RequestInfo | URL, init?: RequestInit, ): Promise => { - const urlStr = input as string; - - // 2. Dialect Mapping (Request) - let body = init?.body; - let isRpc = false; - let rpcId: string | number | undefined; - + const body = init?.body; + // Protocol Detection + // JSON-RPC requests bypass the adapter as they are standard-compliant and + // don't require the dialect translation intended for Vertex AI REST bindings. + // This logic can be removed when a2a-js/sdk is fully compliant. + let effectiveBody = body; if (typeof body === 'string') { try { - let jsonBody = JSON.parse(body); + const jsonBody = JSON.parse(body); - // Unwrap JSON-RPC if present + // If the SDK decided to use JSON-RPC, we bypass the adapter because + // JSON-RPC requests are correctly supported in a2a-js/sdk. if (jsonBody.jsonrpc === '2.0') { - isRpc = true; - rpcId = jsonBody.id; - jsonBody = jsonBody.params; + return await baseFetch(input, init); } - // Apply dialect translation to the message object + // Dialect Mapping (REST / HTTP+JSON) + // Apply translation for Vertex AI Agent Engine compatibility. const message = jsonBody.message || jsonBody; if (message && typeof message === 'object') { // Role: user -> ROLE_USER, agent/model -> ROLE_AGENT @@ -277,23 +276,20 @@ export function createAdapterFetch(baseFetch: typeof fetch): typeof fetch { } } - body = JSON.stringify(jsonBody); + effectiveBody = JSON.stringify(jsonBody); } catch (error) { debugLogger.debug( '[A2AClientManager] Failed to parse request body for dialect translation:', error, ); - // Non-JSON or parse error; let the baseFetch handle it. } } - const response = await baseFetch(urlStr, { ...init, body }); + const response = await baseFetch(input, { ...init, body: effectiveBody }); - // Map response back if (response.ok) { try { const responseData = await response.clone().json(); - const result = responseData.task || responseData.message || responseData; @@ -337,16 +333,6 @@ export function createAdapterFetch(baseFetch: typeof fetch): typeof fetch { result.status.state = mapTaskState(result.status.state); } - if (isRpc) { - return new Response( - JSON.stringify({ - jsonrpc: '2.0', - id: rpcId, - result, - }), - response, - ); - } return new Response(JSON.stringify(result), response); } catch (_e) { // Non-JSON response or unwrapping failure 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 3722ae2e88..9b2bd16aaf 100644 --- a/packages/core/src/agents/delegate-to-agent-tool.test.ts +++ b/packages/core/src/agents/delegate-to-agent-tool.test.ts @@ -21,6 +21,7 @@ vi.mock('./local-invocation.js', () => ({ execute: vi .fn() .mockResolvedValue({ content: [{ type: 'text', text: 'Success' }] }), + shouldConfirmExecute: vi.fn().mockResolvedValue(false), })), })); @@ -29,7 +30,12 @@ vi.mock('./remote-invocation.js', () => ({ execute: vi.fn().mockResolvedValue({ content: [{ type: 'text', text: 'Remote Success' }], }), - shouldConfirmExecute: vi.fn().mockResolvedValue(true), + shouldConfirmExecute: vi.fn().mockResolvedValue({ + type: 'info', + title: 'Remote Confirmation', + prompt: 'Confirm remote call', + onConfirm: vi.fn(), + }), })), })); @@ -219,4 +225,55 @@ describe('DelegateToAgentTool', () => { 'remote_agent', ); }); + + describe('Confirmation', () => { + it('should use default behavior for local agents (super call)', async () => { + const invocation = tool.build({ + agent_name: 'test_agent', + arg1: 'valid', + }); + + // We expect it to call messageBus.publish with 'delegate_to_agent' + // because super.shouldConfirmExecute checks the policy for the tool itself. + await invocation.shouldConfirmExecute(new AbortController().signal); + + expect(messageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_REQUEST, + toolCall: expect.objectContaining({ + name: DELEGATE_TO_AGENT_TOOL_NAME, + }), + }), + ); + }); + + 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(); + }); + }); }); diff --git a/packages/core/src/agents/delegate-to-agent-tool.ts b/packages/core/src/agents/delegate-to-agent-tool.ts index 6ac716b7f4..7f3c4466b5 100644 --- a/packages/core/src/agents/delegate-to-agent-tool.ts +++ b/packages/core/src/agents/delegate-to-agent-tool.ts @@ -12,14 +12,15 @@ import { 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 type { AgentInputs } from './types.js'; type DelegateParams = { agent_name: string } & Record; @@ -166,6 +167,22 @@ class DelegateInvocation extends BaseToolInvocation< return `Delegating to agent '${this.params.agent_name}'`; } + override async shouldConfirmExecute( + abortSignal: AbortSignal, + ): Promise { + const definition = this.registry.getDefinition(this.params.agent_name); + if (!definition || definition.kind !== 'remote') { + return super.shouldConfirmExecute(abortSignal); + } + + 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, @@ -173,26 +190,29 @@ class DelegateInvocation extends BaseToolInvocation< const definition = this.registry.getDefinition(this.params.agent_name); if (!definition) { throw new Error( - `Agent '${this.params.agent_name}' exists in the tool definition but could not be found in the registry.`, + `Agent '${this.params.agent_name}' not found in registry.`, ); } - // Extract arguments (everything except agent_name) - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { agent_name, ...agentArgs } = this.params; + const { agent_name: _agent_name, ...agentArgs } = this.params; + const invocation = this.buildSubInvocation( + definition, + agentArgs as AgentInputs, + ); - // Delegate the creation of the specific invocation (Local or Remote) to the wrapper. - // This centralizes the logic and ensures consistent handling. + return invocation.execute(signal, updateOutput); + } + + private buildSubInvocation( + definition: AgentDefinition, + agentArgs: AgentInputs, + ): ToolInvocation { const wrapper = new SubagentToolWrapper( definition, this.config, this.messageBus, ); - // 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); + return wrapper.build(agentArgs); } } diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index 38b28ffcc7..2e3e810b55 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -12,6 +12,7 @@ import { loadAgentsFromDirectory } from './toml-loader.js'; import { CodebaseInvestigatorAgent } from './codebase-investigator.js'; import { IntrospectionAgent } from './introspection-agent.js'; import { A2AClientManager } from './a2a-client-manager.js'; +import { ADCHandler } from './remote-invocation.js'; import { type z } from 'zod'; import { debugLogger } from '../utils/debugLogger.js'; import { @@ -251,9 +252,12 @@ export class AgentRegistry { // Log remote A2A agent registration for visibility. try { const clientManager = A2AClientManager.getInstance(); + // Use ADCHandler to ensure we can load agents hosted on secure platforms (e.g. Vertex AI) + const authHandler = new ADCHandler(); const agentCard = await clientManager.loadAgent( definition.name, definition.agentCardUrl, + authHandler, ); if (agentCard.skills && agentCard.skills.length > 0) { definition.description = agentCard.skills diff --git a/packages/core/src/agents/remote-invocation.test.ts b/packages/core/src/agents/remote-invocation.test.ts index f3c998e41b..5f2c3eb732 100644 --- a/packages/core/src/agents/remote-invocation.test.ts +++ b/packages/core/src/agents/remote-invocation.test.ts @@ -304,7 +304,7 @@ describe('RemoteAgentInvocation', () => { confirmation.type === 'info' ) { expect(confirmation.title).toContain('Test Agent'); - expect(confirmation.prompt).toContain('http://test-agent/card'); + expect(confirmation.prompt).toContain('Calling remote agent: "hi"'); } else { throw new Error('Expected confirmation to be of type info'); } diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts index 4bc23f7fb1..9acb7794ea 100644 --- a/packages/core/src/agents/remote-invocation.ts +++ b/packages/core/src/agents/remote-invocation.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import type { ToolConfirmationOutcome } from '../tools/tools.js'; import { BaseToolInvocation, type ToolResult, @@ -114,8 +115,10 @@ export class RemoteAgentInvocation extends BaseToolInvocation< return { type: 'info', title: `Call Remote Agent: ${this.definition.displayName ?? this.definition.name}`, - prompt: `This will send a message to the external agent at ${this.definition.agentCardUrl}.`, - onConfirm: async () => {}, // No-op for now, just informational + prompt: `Calling remote agent: "${this.params.query}"`, + onConfirm: async (outcome: ToolConfirmationOutcome) => { + await this.publishPolicyUpdate(outcome); + }, }; }