mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-13 15:40:57 -07:00
feat: add confirmation details support + jsonrpc vs http rest support (#16079)
This commit is contained in:
@@ -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 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -232,25 +232,24 @@ export function createAdapterFetch(baseFetch: typeof fetch): typeof fetch {
|
||||
input: RequestInfo | URL,
|
||||
init?: RequestInit,
|
||||
): Promise<Response> => {
|
||||
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
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
|
||||
@@ -166,6 +167,22 @@ class DelegateInvocation extends BaseToolInvocation<
|
||||
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') {
|
||||
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<AgentInputs, ToolResult> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user