jcarr
/
gemini-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug(core): Do not throw validation errors when building tools in `nonInteractiveToolExecutor`. (#6363)

This commit is contained in:
joshualitt 2025-08-18 13:28:15 -07:00 committed by GitHub
parent 91cd0db2b3
commit d66ddcd82e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 305 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
packages/cli/src/nonInteractiveCli.test.ts
View File

@ -150,7 +150,7 @@ describe('runNonInteractive', () => {
expect(processStdoutSpy).toHaveBeenCalledWith('\n');
});
it('should handle error during tool execution', async () => {
it('should handle error during tool execution and should send error back to the model', async () => {
const toolCallEvent: ServerGeminiStreamEvent = {
type: GeminiEventType.ToolCallRequest,
value: {
@ -162,20 +162,52 @@ describe('runNonInteractive', () => {
},
};
mockCoreExecuteToolCall.mockResolvedValue({
error: new Error('Tool execution failed badly'),
errorType: ToolErrorType.UNHANDLED_EXCEPTION,
error: new Error('Execution failed'),
errorType: ToolErrorType.EXECUTION_FAILED,
responseParts: {
functionResponse: {
name: 'errorTool',
response: {
output: 'Error: Execution failed',
},
},
},
resultDisplay: 'Execution failed',
});
mockGeminiClient.sendMessageStream.mockReturnValue(
createStreamFromEvents([toolCallEvent]),
);
const finalResponse: ServerGeminiStreamEvent[] = [
{
type: GeminiEventType.Content,
value: 'Sorry, let me try again.',
},
];
mockGeminiClient.sendMessageStream
.mockReturnValueOnce(createStreamFromEvents([toolCallEvent]))
.mockReturnValueOnce(createStreamFromEvents(finalResponse));
await runNonInteractive(mockConfig, 'Trigger tool error', 'prompt-id-3');
expect(mockCoreExecuteToolCall).toHaveBeenCalled();
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool errorTool: Tool execution failed badly',
'Error executing tool errorTool: Execution failed',
);
expect(processExitSpy).toHaveBeenCalledWith(1);
expect(processExitSpy).not.toHaveBeenCalled();
expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(2);
expect(mockGeminiClient.sendMessageStream).toHaveBeenNthCalledWith(
2,
[
{
functionResponse: {
name: 'errorTool',
response: {
output: 'Error: Execution failed',
},
},
},
],
expect.any(AbortSignal),
'prompt-id-3',
);
expect(processStdoutSpy).toHaveBeenCalledWith('Sorry, let me try again.');
});
it('should exit with error if sendMessageStream throws initially', async () => {

3
packages/cli/src/nonInteractiveCli.ts
View File

@ -12,7 +12,6 @@ import {
shutdownTelemetry,
isTelemetrySdkInitialized,
GeminiEventType,
ToolErrorType,
parseAndFormatApiError,
} from '@google/gemini-cli-core';
import { Content, Part, FunctionCall } from '@google/genai';
@ -109,8 +108,6 @@ export async function runNonInteractive(
console.error(
`Error executing tool ${fc.name}: ${toolResponse.resultDisplay || toolResponse.error.message}`,
);
if (toolResponse.errorType === ToolErrorType.UNHANDLED_EXCEPTION)
process.exit(1);
}
if (toolResponse.responseParts) {

138
packages/core/src/core/nonInteractiveToolExecutor.test.ts
View File

@ -11,6 +11,7 @@ import {
ToolCallRequestInfo,
ToolResult,
Config,
ToolErrorType,
} from '../index.js';
import { Part } from '@google/genai';
import { MockTool } from '../test-utils/tools.js';
@ -50,7 +51,7 @@ describe('executeToolCall', () => {
returnDisplay: 'Success!',
};
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'buildAndExecute').mockResolvedValue(toolResult);
vi.spyOn(mockTool, 'validateBuildAndExecute').mockResolvedValue(toolResult);
const response = await executeToolCall(
mockConfig,
@ -60,7 +61,7 @@ describe('executeToolCall', () => {
);
expect(mockToolRegistry.getTool).toHaveBeenCalledWith('testTool');
expect(mockTool.buildAndExecute).toHaveBeenCalledWith(
expect(mockTool.validateBuildAndExecute).toHaveBeenCalledWith(
request.args,
abortController.signal,
);
@ -112,17 +113,26 @@ describe('executeToolCall', () => {
]);
});
it('should return an error if tool execution fails', async () => {
it('should return an error if tool validation fails', async () => {
const request: ToolCallRequestInfo = {
callId: 'call3',
name: 'testTool',
args: { param1: 'value1' },
args: { param1: 'invalid' },
isClientInitiated: false,
prompt_id: 'prompt-id-3',
};
const executionError = new Error('Tool execution failed');
const validationErrorResult: ToolResult = {
llmContent: 'Error: Invalid parameters',
returnDisplay: 'Invalid parameters',
error: {
message: 'Invalid parameters',
type: ToolErrorType.INVALID_TOOL_PARAMS,
},
};
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'buildAndExecute').mockRejectedValue(executionError);
vi.spyOn(mockTool, 'validateBuildAndExecute').mockResolvedValue(
validationErrorResult,
);
const response = await executeToolCall(
mockConfig,
@ -130,22 +140,24 @@ describe('executeToolCall', () => {
mockToolRegistry,
abortController.signal,
);
expect(response.callId).toBe('call3');
expect(response.error).toBe(executionError);
expect(response.resultDisplay).toBe('Tool execution failed');
expect(response.responseParts).toEqual([
{
expect(response).toStrictEqual({
callId: 'call3',
error: new Error('Invalid parameters'),
errorType: ToolErrorType.INVALID_TOOL_PARAMS,
responseParts: {
functionResponse: {
name: 'testTool',
id: 'call3',
response: { error: 'Tool execution failed' },
name: 'testTool',
response: {
output: 'Error: Invalid parameters',
},
},
]);
},
resultDisplay: 'Invalid parameters',
});
});
it('should handle cancellation during tool execution', async () => {
it('should return an error if tool execution fails', async () => {
const request: ToolCallRequestInfo = {
callId: 'call4',
name: 'testTool',
@ -153,32 +165,56 @@ describe('executeToolCall', () => {
isClientInitiated: false,
prompt_id: 'prompt-id-4',
};
const cancellationError = new Error('Operation cancelled');
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'buildAndExecute').mockImplementation(
async (_args, signal) => {
if (signal?.aborted) {
return Promise.reject(cancellationError);
}
return new Promise((_resolve, reject) => {
signal?.addEventListener('abort', () => {
reject(cancellationError);
});
// Simulate work that might happen if not aborted immediately
const timeoutId = setTimeout(
() =>
reject(
new Error('Should have been cancelled if not aborted prior'),
),
100,
);
signal?.addEventListener('abort', () => clearTimeout(timeoutId));
});
const executionErrorResult: ToolResult = {
llmContent: 'Error: Execution failed',
returnDisplay: 'Execution failed',
error: {
message: 'Execution failed',
type: ToolErrorType.EXECUTION_FAILED,
},
};
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'validateBuildAndExecute').mockResolvedValue(
executionErrorResult,
);
const response = await executeToolCall(
mockConfig,
request,
mockToolRegistry,
abortController.signal,
);
expect(response).toStrictEqual({
callId: 'call4',
error: new Error('Execution failed'),
errorType: ToolErrorType.EXECUTION_FAILED,
responseParts: {
functionResponse: {
id: 'call4',
name: 'testTool',
response: {
output: 'Error: Execution failed',
},
},
},
resultDisplay: 'Execution failed',
});
});
it('should return an unhandled exception error if execution throws', async () => {
const request: ToolCallRequestInfo = {
callId: 'call5',
name: 'testTool',
args: { param1: 'value1' },
isClientInitiated: false,
prompt_id: 'prompt-id-5',
};
const executionError = new Error('Something went very wrong');
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'validateBuildAndExecute').mockRejectedValue(
executionError,
);
abortController.abort(); // Abort before calling
const response = await executeToolCall(
mockConfig,
request,
@ -186,18 +222,28 @@ describe('executeToolCall', () => {
abortController.signal,
);
expect(response.callId).toBe('call4');
expect(response.error?.message).toBe(cancellationError.message);
expect(response.resultDisplay).toBe('Operation cancelled');
expect(response.callId).toBe('call5');
expect(response.error).toBe(executionError);
expect(response.errorType).toBe(ToolErrorType.UNHANDLED_EXCEPTION);
expect(response.resultDisplay).toBe('Something went very wrong');
expect(response.responseParts).toEqual([
{
functionResponse: {
name: 'testTool',
id: 'call5',
response: { error: 'Something went very wrong' },
},
},
]);
});
it('should correctly format llmContent with inlineData', async () => {
const request: ToolCallRequestInfo = {
callId: 'call5',
callId: 'call6',
name: 'testTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-id-5',
prompt_id: 'prompt-id-6',
};
const imageDataPart: Part = {
inlineData: { mimeType: 'image/png', data: 'base64data' },
@ -207,7 +253,7 @@ describe('executeToolCall', () => {
returnDisplay: 'Image processed',
};
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
vi.spyOn(mockTool, 'buildAndExecute').mockResolvedValue(toolResult);
vi.spyOn(mockTool, 'validateBuildAndExecute').mockResolvedValue(toolResult);
const response = await executeToolCall(
mockConfig,
@ -221,7 +267,7 @@ describe('executeToolCall', () => {
{
functionResponse: {
name: 'testTool',
id: 'call5',
id: 'call6',
response: {
output: 'Binary content of type image/png was processed.',
},

2
packages/core/src/core/nonInteractiveToolExecutor.ts
View File

@ -66,7 +66,7 @@ export async function executeToolCall(
try {
// Directly execute without confirmation or live output handling
const effectiveAbortSignal = abortSignal ?? new AbortController().signal;
const toolResult: ToolResult = await tool.buildAndExecute(
const toolResult: ToolResult = await tool.validateBuildAndExecute(
toolCallRequest.args,
effectiveAbortSignal,
// No live output callback for non-interactive mode

1
packages/core/src/tools/tool-error.ts
View File

@ -13,6 +13,7 @@ export enum ToolErrorType {
UNKNOWN = 'unknown',
UNHANDLED_EXCEPTION = 'unhandled_exception',
TOOL_NOT_REGISTERED = 'tool_not_registered',
EXECUTION_FAILED = 'execution_failed',
// File System Errors
FILE_NOT_FOUND = 'file_not_found',

115
packages/core/src/tools/tools.test.ts
View File

@ -4,8 +4,119 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { hasCycleInSchema } from './tools.js'; // Added getStringifiedResultForDisplay
import { describe, it, expect, vi } from 'vitest';
import {
DeclarativeTool,
hasCycleInSchema,
Kind,
ToolInvocation,
ToolResult,
} from './tools.js';
import { ToolErrorType } from './tool-error.js';
class TestToolInvocation implements ToolInvocation<object, ToolResult> {
constructor(
readonly params: object,
private readonly executeFn: () => Promise<ToolResult>,
) {}
getDescription(): string {
return 'A test invocation';
}
toolLocations() {
return [];
}
shouldConfirmExecute(): Promise<false> {
return Promise.resolve(false);
}
execute(): Promise<ToolResult> {
return this.executeFn();
}
}
class TestTool extends DeclarativeTool<object, ToolResult> {
private readonly buildFn: (params: object) => TestToolInvocation;
constructor(buildFn: (params: object) => TestToolInvocation) {
super('test-tool', 'Test Tool', 'A tool for testing', Kind.Other, {});
this.buildFn = buildFn;
}
build(params: object): ToolInvocation<object, ToolResult> {
return this.buildFn(params);
}
}
describe('DeclarativeTool', () => {
describe('validateBuildAndExecute', () => {
const abortSignal = new AbortController().signal;
it('should return INVALID_TOOL_PARAMS error if build fails', async () => {
const buildError = new Error('Invalid build parameters');
const buildFn = vi.fn().mockImplementation(() => {
throw buildError;
});
const tool = new TestTool(buildFn);
const params = { foo: 'bar' };
const result = await tool.validateBuildAndExecute(params, abortSignal);
expect(buildFn).toHaveBeenCalledWith(params);
expect(result).toEqual({
llmContent: `Error: Invalid parameters provided. Reason: ${buildError.message}`,
returnDisplay: buildError.message,
error: {
message: buildError.message,
type: ToolErrorType.INVALID_TOOL_PARAMS,
},
});
});
it('should return EXECUTION_FAILED error if execute fails', async () => {
const executeError = new Error('Execution failed');
const executeFn = vi.fn().mockRejectedValue(executeError);
const invocation = new TestToolInvocation({}, executeFn);
const buildFn = vi.fn().mockReturnValue(invocation);
const tool = new TestTool(buildFn);
const params = { foo: 'bar' };
const result = await tool.validateBuildAndExecute(params, abortSignal);
expect(buildFn).toHaveBeenCalledWith(params);
expect(executeFn).toHaveBeenCalled();
expect(result).toEqual({
llmContent: `Error: Tool call execution failed. Reason: ${executeError.message}`,
returnDisplay: executeError.message,
error: {
message: executeError.message,
type: ToolErrorType.EXECUTION_FAILED,
},
});
});
it('should return the result of execute on success', async () => {
const successResult: ToolResult = {
llmContent: 'Success!',
returnDisplay: 'Success!',
summary: 'Tool executed successfully',
};
const executeFn = vi.fn().mockResolvedValue(successResult);
const invocation = new TestToolInvocation({}, executeFn);
const buildFn = vi.fn().mockReturnValue(invocation);
const tool = new TestTool(buildFn);
const params = { foo: 'bar' };
const result = await tool.validateBuildAndExecute(params, abortSignal);
expect(buildFn).toHaveBeenCalledWith(params);
expect(executeFn).toHaveBeenCalled();
expect(result).toEqual(successResult);
});
});
});
describe('hasCycleInSchema', () => {
it('should detect a simple direct cycle', () => {

58
packages/core/src/tools/tools.ts
View File

@ -200,6 +200,64 @@ export abstract class DeclarativeTool<
const invocation = this.build(params);
return invocation.execute(signal, updateOutput);
}
/**
* Similar to `build` but never throws.
* @param params The raw, untrusted parameters from the model.
* @returns A `ToolInvocation` instance.
*/
private silentBuild(
params: TParams,
): ToolInvocation<TParams, TResult> | Error {
try {
return this.build(params);
} catch (e) {
if (e instanceof Error) {
return e;
}
return new Error(String(e));
}
}
/**
* A convenience method that builds and executes the tool in one step.
* Never throws.
* @param params The raw, untrusted parameters from the model.
* @params abortSignal a signal to abort.
* @returns The result of the tool execution.
*/
async validateBuildAndExecute(
params: TParams,
abortSignal: AbortSignal,
): Promise<ToolResult> {
const invocationOrError = this.silentBuild(params);
if (invocationOrError instanceof Error) {
const errorMessage = invocationOrError.message;
return {
llmContent: `Error: Invalid parameters provided. Reason: ${errorMessage}`,
returnDisplay: errorMessage,
error: {
message: errorMessage,
type: ToolErrorType.INVALID_TOOL_PARAMS,
},
};
}
try {
return await invocationOrError.execute(abortSignal);
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
return {
llmContent: `Error: Tool call execution failed. Reason: ${errorMessage}`,
returnDisplay: errorMessage,
error: {
message: errorMessage,
type: ToolErrorType.EXECUTION_FAILED,
},
};
}
}
}
/**