mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-12 12:54:07 -07:00
fix(browser): fix duplicate function declaration error in browser agent (#22207)
This commit is contained in:
@@ -2495,4 +2495,337 @@ describe('LocalAgentExecutor', () => {
|
|||||||
expect(mockSetHistory).toHaveBeenCalledWith(compressedHistory);
|
expect(mockSetHistory).toHaveBeenCalledWith(compressedHistory);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('DeclarativeTool instance tools (browser agent pattern)', () => {
|
||||||
|
/**
|
||||||
|
* The browser agent passes DeclarativeTool instances (not string names) in
|
||||||
|
* toolConfig.tools. These tests ensure that prepareToolsList() and
|
||||||
|
* create() handle that pattern correctly — in particular, that each tool
|
||||||
|
* appears exactly once in the function declarations sent to the model.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Helper that creates a definition using MockTool *instances* in
|
||||||
|
* toolConfig.tools — the same pattern the browser agent uses.
|
||||||
|
*/
|
||||||
|
const createInstanceToolDefinition = (
|
||||||
|
instanceTools: MockTool[],
|
||||||
|
outputConfigMode: 'default' | 'none' = 'default',
|
||||||
|
): LocalAgentDefinition => {
|
||||||
|
const outputConfig =
|
||||||
|
outputConfigMode === 'default'
|
||||||
|
? {
|
||||||
|
outputName: 'finalResult',
|
||||||
|
description: 'The final result.',
|
||||||
|
schema: z.string(),
|
||||||
|
}
|
||||||
|
: undefined;
|
||||||
|
|
||||||
|
return {
|
||||||
|
kind: 'local',
|
||||||
|
name: 'BrowserLikeAgent',
|
||||||
|
description: 'An agent using instance tools.',
|
||||||
|
inputConfig: {
|
||||||
|
inputSchema: {
|
||||||
|
type: 'object',
|
||||||
|
properties: {
|
||||||
|
goal: { type: 'string', description: 'goal' },
|
||||||
|
},
|
||||||
|
required: ['goal'],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
modelConfig: {
|
||||||
|
model: 'gemini-test-model',
|
||||||
|
generateContentConfig: { temperature: 0, topP: 1 },
|
||||||
|
},
|
||||||
|
runConfig: { maxTimeMinutes: 5, maxTurns: 5 },
|
||||||
|
promptConfig: { systemPrompt: 'Achieve: ${goal}.' },
|
||||||
|
toolConfig: {
|
||||||
|
// Cast required because the type expects AnyDeclarativeTool |
|
||||||
|
// string | FunctionDeclaration; MockTool satisfies the first.
|
||||||
|
tools: instanceTools as unknown as AnyDeclarativeTool[],
|
||||||
|
},
|
||||||
|
outputConfig,
|
||||||
|
} as unknown as LocalAgentDefinition;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Helper to extract the functionDeclarations sent to GeminiChat.
|
||||||
|
*/
|
||||||
|
const getSentFunctionDeclarations = () => {
|
||||||
|
const chatCtorArgs = MockedGeminiChat.mock.calls[0];
|
||||||
|
const toolsArg = chatCtorArgs[2] as Tool[];
|
||||||
|
return toolsArg[0].functionDeclarations ?? [];
|
||||||
|
};
|
||||||
|
|
||||||
|
it('should produce NO duplicate function declarations when tools are DeclarativeTool instances', async () => {
|
||||||
|
const clickTool = new MockTool({ name: 'click' });
|
||||||
|
const fillTool = new MockTool({ name: 'fill' });
|
||||||
|
const snapshotTool = new MockTool({ name: 'take_snapshot' });
|
||||||
|
|
||||||
|
const definition = createInstanceToolDefinition([
|
||||||
|
clickTool,
|
||||||
|
fillTool,
|
||||||
|
snapshotTool,
|
||||||
|
]);
|
||||||
|
|
||||||
|
mockModelResponse([
|
||||||
|
{
|
||||||
|
name: TASK_COMPLETE_TOOL_NAME,
|
||||||
|
args: { finalResult: 'done' },
|
||||||
|
id: 'c1',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
await executor.run({ goal: 'Test' }, signal);
|
||||||
|
|
||||||
|
const declarations = getSentFunctionDeclarations();
|
||||||
|
const names = declarations.map((d) => d.name);
|
||||||
|
|
||||||
|
// Each tool must appear exactly once
|
||||||
|
expect(names.filter((n) => n === 'click')).toHaveLength(1);
|
||||||
|
expect(names.filter((n) => n === 'fill')).toHaveLength(1);
|
||||||
|
expect(names.filter((n) => n === 'take_snapshot')).toHaveLength(1);
|
||||||
|
|
||||||
|
// Total = 3 tools + complete_task
|
||||||
|
expect(declarations).toHaveLength(4);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should register DeclarativeTool instances in the isolated tool registry', async () => {
|
||||||
|
const clickTool = new MockTool({ name: 'click' });
|
||||||
|
const navTool = new MockTool({ name: 'navigate_page' });
|
||||||
|
|
||||||
|
const definition = createInstanceToolDefinition([clickTool, navTool]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
|
||||||
|
const registry = executor['toolRegistry'];
|
||||||
|
expect(registry.getTool('click')).toBeDefined();
|
||||||
|
expect(registry.getTool('navigate_page')).toBeDefined();
|
||||||
|
// Should NOT have tools that were not passed
|
||||||
|
expect(registry.getTool(LS_TOOL_NAME)).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should handle mixed string + DeclarativeTool instances without duplicates', async () => {
|
||||||
|
const instanceTool = new MockTool({ name: 'fill' });
|
||||||
|
|
||||||
|
const definition: LocalAgentDefinition = {
|
||||||
|
kind: 'local',
|
||||||
|
name: 'MixedAgent',
|
||||||
|
description: 'Uses both patterns.',
|
||||||
|
inputConfig: {
|
||||||
|
inputSchema: {
|
||||||
|
type: 'object',
|
||||||
|
properties: { goal: { type: 'string', description: 'goal' } },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
modelConfig: {
|
||||||
|
model: 'gemini-test-model',
|
||||||
|
generateContentConfig: { temperature: 0, topP: 1 },
|
||||||
|
},
|
||||||
|
runConfig: { maxTimeMinutes: 5, maxTurns: 5 },
|
||||||
|
promptConfig: { systemPrompt: 'Achieve: ${goal}.' },
|
||||||
|
toolConfig: {
|
||||||
|
tools: [
|
||||||
|
LS_TOOL_NAME, // string reference
|
||||||
|
instanceTool as unknown as AnyDeclarativeTool, // instance
|
||||||
|
],
|
||||||
|
},
|
||||||
|
outputConfig: {
|
||||||
|
outputName: 'finalResult',
|
||||||
|
description: 'result',
|
||||||
|
schema: z.string(),
|
||||||
|
},
|
||||||
|
} as unknown as LocalAgentDefinition;
|
||||||
|
|
||||||
|
mockModelResponse([
|
||||||
|
{
|
||||||
|
name: TASK_COMPLETE_TOOL_NAME,
|
||||||
|
args: { finalResult: 'ok' },
|
||||||
|
id: 'c1',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
await executor.run({ goal: 'Mixed' }, signal);
|
||||||
|
|
||||||
|
const declarations = getSentFunctionDeclarations();
|
||||||
|
const names = declarations.map((d) => d.name);
|
||||||
|
|
||||||
|
expect(names.filter((n) => n === LS_TOOL_NAME)).toHaveLength(1);
|
||||||
|
expect(names.filter((n) => n === 'fill')).toHaveLength(1);
|
||||||
|
expect(names.filter((n) => n === TASK_COMPLETE_TOOL_NAME)).toHaveLength(
|
||||||
|
1,
|
||||||
|
);
|
||||||
|
// Total = ls + fill + complete_task
|
||||||
|
expect(declarations).toHaveLength(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should correctly execute tools passed as DeclarativeTool instances', async () => {
|
||||||
|
const executeFn = vi.fn().mockResolvedValue({
|
||||||
|
llmContent: 'Clicked successfully.',
|
||||||
|
returnDisplay: 'Clicked successfully.',
|
||||||
|
});
|
||||||
|
const clickTool = new MockTool({ name: 'click', execute: executeFn });
|
||||||
|
|
||||||
|
const definition = createInstanceToolDefinition([clickTool]);
|
||||||
|
|
||||||
|
// Turn 1: Model calls click
|
||||||
|
mockModelResponse([
|
||||||
|
{ name: 'click', args: { uid: '42' }, id: 'call-click' },
|
||||||
|
]);
|
||||||
|
mockScheduleAgentTools.mockResolvedValueOnce([
|
||||||
|
{
|
||||||
|
status: 'success',
|
||||||
|
request: {
|
||||||
|
callId: 'call-click',
|
||||||
|
name: 'click',
|
||||||
|
args: { uid: '42' },
|
||||||
|
isClientInitiated: false,
|
||||||
|
prompt_id: 'test',
|
||||||
|
},
|
||||||
|
tool: {} as AnyDeclarativeTool,
|
||||||
|
invocation: {} as AnyToolInvocation,
|
||||||
|
response: {
|
||||||
|
callId: 'call-click',
|
||||||
|
resultDisplay: 'Clicked',
|
||||||
|
responseParts: [
|
||||||
|
{
|
||||||
|
functionResponse: {
|
||||||
|
name: 'click',
|
||||||
|
response: { result: 'Clicked' },
|
||||||
|
id: 'call-click',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
error: undefined,
|
||||||
|
errorType: undefined,
|
||||||
|
contentLength: undefined,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Turn 2: Model completes
|
||||||
|
mockModelResponse([
|
||||||
|
{
|
||||||
|
name: TASK_COMPLETE_TOOL_NAME,
|
||||||
|
args: { finalResult: 'done' },
|
||||||
|
id: 'call-done',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
const output = await executor.run({ goal: 'Click test' }, signal);
|
||||||
|
|
||||||
|
// The scheduler should have received the click tool call
|
||||||
|
expect(mockScheduleAgentTools).toHaveBeenCalled();
|
||||||
|
const scheduledRequests = mockScheduleAgentTools.mock
|
||||||
|
.calls[0][1] as ToolCallRequestInfo[];
|
||||||
|
expect(scheduledRequests).toHaveLength(1);
|
||||||
|
expect(scheduledRequests[0].name).toBe('click');
|
||||||
|
|
||||||
|
expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should always include complete_task even when all tools are instances', async () => {
|
||||||
|
const definition = createInstanceToolDefinition(
|
||||||
|
[new MockTool({ name: 'take_snapshot' })],
|
||||||
|
'none',
|
||||||
|
);
|
||||||
|
|
||||||
|
mockModelResponse([
|
||||||
|
{
|
||||||
|
name: TASK_COMPLETE_TOOL_NAME,
|
||||||
|
args: { result: 'done' },
|
||||||
|
id: 'c1',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
await executor.run({ goal: 'Test' }, signal);
|
||||||
|
|
||||||
|
const declarations = getSentFunctionDeclarations();
|
||||||
|
const names = declarations.map((d) => d.name);
|
||||||
|
|
||||||
|
expect(names).toContain(TASK_COMPLETE_TOOL_NAME);
|
||||||
|
expect(names).toContain('take_snapshot');
|
||||||
|
expect(declarations).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should produce unique declarations for many instance tools (browser agent scale)', async () => {
|
||||||
|
// Simulates the full set of tools the browser agent typically registers
|
||||||
|
const browserToolNames = [
|
||||||
|
'click',
|
||||||
|
'click_at',
|
||||||
|
'fill',
|
||||||
|
'fill_form',
|
||||||
|
'hover',
|
||||||
|
'drag',
|
||||||
|
'press_key',
|
||||||
|
'take_snapshot',
|
||||||
|
'navigate_page',
|
||||||
|
'new_page',
|
||||||
|
'close_page',
|
||||||
|
'select_page',
|
||||||
|
'evaluate_script',
|
||||||
|
'type_text',
|
||||||
|
];
|
||||||
|
const instanceTools = browserToolNames.map(
|
||||||
|
(name) => new MockTool({ name }),
|
||||||
|
);
|
||||||
|
|
||||||
|
const definition = createInstanceToolDefinition(instanceTools);
|
||||||
|
|
||||||
|
mockModelResponse([
|
||||||
|
{
|
||||||
|
name: TASK_COMPLETE_TOOL_NAME,
|
||||||
|
args: { finalResult: 'done' },
|
||||||
|
id: 'c1',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const executor = await LocalAgentExecutor.create(
|
||||||
|
definition,
|
||||||
|
mockConfig,
|
||||||
|
onActivity,
|
||||||
|
);
|
||||||
|
await executor.run({ goal: 'Scale test' }, signal);
|
||||||
|
|
||||||
|
const declarations = getSentFunctionDeclarations();
|
||||||
|
const names = declarations.map((d) => d.name);
|
||||||
|
|
||||||
|
// Every tool name must appear exactly once
|
||||||
|
for (const toolName of browserToolNames) {
|
||||||
|
const count = names.filter((n) => n === toolName).length;
|
||||||
|
expect(count).toBe(1);
|
||||||
|
}
|
||||||
|
// Plus complete_task
|
||||||
|
expect(declarations).toHaveLength(browserToolNames.length + 1);
|
||||||
|
|
||||||
|
// Verify the complete set of names has no duplicates
|
||||||
|
const uniqueNames = new Set(names);
|
||||||
|
expect(uniqueNames.size).toBe(names.length);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user