fix(cli): record tool calls in non-interactive mode (#10951)

This commit is contained in:
bl-ue
2025-10-14 09:51:00 -06:00
committed by GitHub
parent 3ba4ba79fa
commit 9e8c767694
8 changed files with 316 additions and 98 deletions

View File

@@ -9,6 +9,8 @@ import type {
ToolRegistry,
ServerGeminiStreamEvent,
SessionMetrics,
AnyDeclarativeTool,
AnyToolInvocation,
} from '@google/gemini-cli-core';
import {
executeToolCall,
@@ -204,7 +206,25 @@ describe('runNonInteractive', () => {
},
};
const toolResponse: Part[] = [{ text: 'Tool response' }];
mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse });
mockCoreExecuteToolCall.mockResolvedValue({
status: 'success',
request: {
callId: 'tool-1',
name: 'testTool',
args: { arg1: 'value1' },
isClientInitiated: false,
prompt_id: 'prompt-id-2',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
responseParts: toolResponse,
callId: 'tool-1',
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
const firstCallEvents: ServerGeminiStreamEvent[] = [toolCallEvent];
const secondCallEvents: ServerGeminiStreamEvent[] = [
@@ -254,19 +274,32 @@ describe('runNonInteractive', () => {
},
};
mockCoreExecuteToolCall.mockResolvedValue({
error: new Error('Execution failed'),
errorType: ToolErrorType.EXECUTION_FAILED,
responseParts: [
{
functionResponse: {
name: 'errorTool',
response: {
output: 'Error: Execution failed',
status: 'error',
request: {
callId: 'tool-1',
name: 'errorTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-id-3',
},
tool: {} as AnyDeclarativeTool,
response: {
callId: 'tool-1',
error: new Error('Execution failed'),
errorType: ToolErrorType.EXECUTION_FAILED,
responseParts: [
{
functionResponse: {
name: 'errorTool',
response: {
output: 'Error: Execution failed',
},
},
},
},
],
resultDisplay: 'Execution failed',
],
resultDisplay: 'Execution failed',
contentLength: undefined,
},
});
const finalResponse: ServerGeminiStreamEvent[] = [
{
@@ -340,9 +373,22 @@ describe('runNonInteractive', () => {
},
};
mockCoreExecuteToolCall.mockResolvedValue({
error: new Error('Tool "nonexistentTool" not found in registry.'),
resultDisplay: 'Tool "nonexistentTool" not found in registry.',
responseParts: [],
status: 'error',
request: {
callId: 'tool-1',
name: 'nonexistentTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-id-5',
},
response: {
callId: 'tool-1',
error: new Error('Tool "nonexistentTool" not found in registry.'),
resultDisplay: 'Tool "nonexistentTool" not found in registry.',
responseParts: [],
errorType: undefined,
contentLength: undefined,
},
});
const finalResponse: ServerGeminiStreamEvent[] = [
{
@@ -501,7 +547,25 @@ describe('runNonInteractive', () => {
},
};
const toolResponse: Part[] = [{ text: 'Tool executed successfully' }];
mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse });
mockCoreExecuteToolCall.mockResolvedValue({
status: 'success',
request: {
callId: 'tool-1',
name: 'testTool',
args: { arg1: 'value1' },
isClientInitiated: false,
prompt_id: 'prompt-id-tool-only',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
responseParts: toolResponse,
callId: 'tool-1',
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
// First call returns only tool call, no content
const firstCallEvents: ServerGeminiStreamEvent[] = [
@@ -897,7 +961,25 @@ describe('runNonInteractive', () => {
},
};
const toolResponse: Part[] = [{ text: 'file.txt' }];
mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse });
mockCoreExecuteToolCall.mockResolvedValue({
status: 'success',
request: {
callId: 'tool-shell-1',
name: 'ShellTool',
args: { command: 'ls' },
isClientInitiated: false,
prompt_id: 'prompt-id-allowed',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
responseParts: toolResponse,
callId: 'tool-shell-1',
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
const firstCallEvents: ServerGeminiStreamEvent[] = [toolCallEvent];
const secondCallEvents: ServerGeminiStreamEvent[] = [

View File

@@ -4,7 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import type { Config, ToolCallRequestInfo } from '@google/gemini-cli-core';
import type {
Config,
ToolCallRequestInfo,
CompletedToolCall,
} from '@google/gemini-cli-core';
import { isSlashCommand } from './ui/utils/commandUtils.js';
import type { LoadedSettings } from './config/settings.js';
import {
@@ -132,12 +136,17 @@ export async function runNonInteractive(
if (toolCallRequests.length > 0) {
const toolResponseParts: Part[] = [];
const completedToolCalls: CompletedToolCall[] = [];
for (const requestInfo of toolCallRequests) {
const toolResponse = await executeToolCall(
const completedToolCall = await executeToolCall(
config,
requestInfo,
abortController.signal,
);
const toolResponse = completedToolCall.response;
completedToolCalls.push(completedToolCall);
if (toolResponse.error) {
handleToolError(
@@ -155,6 +164,20 @@ export async function runNonInteractive(
toolResponseParts.push(...toolResponse.responseParts);
}
}
// Record tool calls with full metadata before sending responses to Gemini
try {
const currentModel =
geminiClient.getCurrentSequenceModel() ?? config.getModel();
geminiClient
.getChat()
.recordCompletedToolCalls(currentModel, completedToolCalls);
} catch (error) {
console.error(
`Error recording completed tool call information: ${error}`,
);
}
currentMessages = [{ role: 'user', parts: toolResponseParts }];
} else {
if (config.getOutputFormat() === OutputFormat.JSON) {

View File

@@ -35,6 +35,7 @@ import type {
OutputConfig,
} from './types.js';
import { AgentTerminateMode } from './types.js';
import type { AnyDeclarativeTool, AnyToolInvocation } from '../tools/tools.js';
const { mockSendMessageStream, mockExecuteToolCall } = vi.hoisted(() => ({
mockSendMessageStream: vi.fn(),
@@ -318,18 +319,32 @@ describe('AgentExecutor', () => {
'T1: Listing',
);
mockExecuteToolCall.mockResolvedValueOnce({
callId: 'call1',
resultDisplay: 'file1.txt',
responseParts: [
{
functionResponse: {
name: LSTool.Name,
response: { result: 'file1.txt' },
id: 'call1',
status: 'success',
request: {
callId: 'call1',
name: LSTool.Name,
args: { path: '.' },
isClientInitiated: false,
prompt_id: 'test-prompt',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: 'call1',
resultDisplay: 'file1.txt',
responseParts: [
{
functionResponse: {
name: LSTool.Name,
response: { result: 'file1.txt' },
id: 'call1',
},
},
},
],
error: undefined,
],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
// Turn 2: Model calls complete_task with required output
@@ -451,13 +466,32 @@ describe('AgentExecutor', () => {
{ name: LSTool.Name, args: { path: '.' }, id: 'call1' },
]);
mockExecuteToolCall.mockResolvedValueOnce({
callId: 'call1',
resultDisplay: 'ok',
responseParts: [
{
functionResponse: { name: LSTool.Name, response: {}, id: 'call1' },
},
],
status: 'success',
request: {
callId: 'call1',
name: LSTool.Name,
args: { path: '.' },
isClientInitiated: false,
prompt_id: 'test-prompt',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: 'call1',
resultDisplay: 'ok',
responseParts: [
{
functionResponse: {
name: LSTool.Name,
response: {},
id: 'call1',
},
},
],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
mockModelResponse(
@@ -504,13 +538,32 @@ describe('AgentExecutor', () => {
{ name: LSTool.Name, args: { path: '.' }, id: 'call1' },
]);
mockExecuteToolCall.mockResolvedValueOnce({
callId: 'call1',
resultDisplay: 'ok',
responseParts: [
{
functionResponse: { name: LSTool.Name, response: {}, id: 'call1' },
},
],
status: 'success',
request: {
callId: 'call1',
name: LSTool.Name,
args: { path: '.' },
isClientInitiated: false,
prompt_id: 'test-prompt',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: 'call1',
resultDisplay: 'ok',
responseParts: [
{
functionResponse: {
name: LSTool.Name,
response: {},
id: 'call1',
},
},
],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
mockModelResponse([], 'I think I am done.');
@@ -675,17 +728,26 @@ describe('AgentExecutor', () => {
if (callsStarted === 2) resolveCalls();
await vi.advanceTimersByTimeAsync(100);
return {
callId: reqInfo.callId,
resultDisplay: 'ok',
responseParts: [
{
functionResponse: {
name: reqInfo.name,
response: {},
id: reqInfo.callId,
status: 'success',
request: reqInfo,
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: reqInfo.callId,
resultDisplay: 'ok',
responseParts: [
{
functionResponse: {
name: reqInfo.name,
response: {},
id: reqInfo.callId,
},
},
},
],
],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
};
});
@@ -802,11 +864,26 @@ describe('AgentExecutor', () => {
const mockWorkResponse = (id: string) => {
mockModelResponse([{ name: LSTool.Name, args: { path: '.' }, id }]);
mockExecuteToolCall.mockResolvedValueOnce({
callId: id,
resultDisplay: 'ok',
responseParts: [
{ functionResponse: { name: LSTool.Name, response: {}, id } },
],
status: 'success',
request: {
callId: id,
name: LSTool.Name,
args: { path: '.' },
isClientInitiated: false,
prompt_id: 'test-prompt',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: id,
resultDisplay: 'ok',
responseParts: [
{ functionResponse: { name: LSTool.Name, response: {}, id } },
],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
};
@@ -835,12 +912,21 @@ describe('AgentExecutor', () => {
mockModelResponse([{ name: LSTool.Name, args: { path: '.' }, id: 't1' }]);
// Long running tool
mockExecuteToolCall.mockImplementationOnce(async () => {
mockExecuteToolCall.mockImplementationOnce(async (_ctx, reqInfo) => {
await vi.advanceTimersByTimeAsync(61 * 1000);
return {
callId: 't1',
resultDisplay: 'ok',
responseParts: [],
status: 'success',
request: reqInfo,
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: 't1',
resultDisplay: 'ok',
responseParts: [],
error: undefined,
errorType: undefined,
contentLength: undefined,
},
};
});

View File

@@ -534,7 +534,7 @@ export class AgentExecutor<TOutput extends z.ZodTypeAny> {
// Create a promise for the tool execution
const executionPromise = (async () => {
const toolResponse = await executeToolCall(
const { response: toolResponse } = await executeToolCall(
this.runtimeContext,
requestInfo,
signal,

View File

@@ -82,7 +82,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockResolvedValue(toolResult);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -126,7 +126,7 @@ describe('executeToolCall', () => {
'anotherTool',
]);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -167,7 +167,7 @@ describe('executeToolCall', () => {
throw new Error('Invalid parameters');
});
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -212,7 +212,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockResolvedValue(executionErrorResult);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -248,7 +248,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockRejectedValue(new Error('Something went very wrong'));
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -290,7 +290,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockResolvedValue(toolResult);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -333,7 +333,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockResolvedValue(toolResult);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,
@@ -361,7 +361,7 @@ describe('executeToolCall', () => {
vi.mocked(mockToolRegistry.getTool).mockReturnValue(mockTool);
executeFn.mockResolvedValue(toolResult);
const response = await executeToolCall(
const { response } = await executeToolCall(
mockConfig,
request,
abortController.signal,

View File

@@ -4,12 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import type {
ToolCallRequestInfo,
ToolCallResponseInfo,
Config,
} from '../index.js';
import { CoreToolScheduler } from './coreToolScheduler.js';
import type { ToolCallRequestInfo, Config } from '../index.js';
import {
CoreToolScheduler,
type CompletedToolCall,
} from './coreToolScheduler.js';
/**
* Executes a single tool call non-interactively by leveraging the CoreToolScheduler.
@@ -18,14 +17,14 @@ export async function executeToolCall(
config: Config,
toolCallRequest: ToolCallRequestInfo,
abortSignal: AbortSignal,
): Promise<ToolCallResponseInfo> {
return new Promise<ToolCallResponseInfo>((resolve, reject) => {
): Promise<CompletedToolCall> {
return new Promise<CompletedToolCall>((resolve, reject) => {
new CoreToolScheduler({
config,
getPreferredEditor: () => undefined,
onEditorClose: () => {},
onAllToolCallsComplete: async (completedToolCalls) => {
resolve(completedToolCalls[0].response);
resolve(completedToolCalls[0]);
},
})
.schedule(toolCallRequest, abortSignal)

View File

@@ -36,6 +36,7 @@ import type {
GenerateContentResponse,
} from '@google/genai';
import { ToolErrorType } from '../tools/tool-error.js';
import type { AnyDeclarativeTool, AnyToolInvocation } from '../tools/tools.js';
vi.mock('./geminiChat.js');
vi.mock('./contentGenerator.js');
@@ -580,13 +581,26 @@ describe('subagent.ts', () => {
]),
);
// Mock the tool execution result
// Mock the tool execution result - must return CompletedToolCall
vi.mocked(executeToolCall).mockResolvedValue({
callId: 'call_1',
responseParts: [{ text: 'file1.txt\nfile2.ts' }],
resultDisplay: 'Listed 2 files',
error: undefined,
errorType: undefined, // Or ToolErrorType.NONE if available and appropriate
status: 'success',
request: {
callId: 'call_1',
name: 'list_files',
args: { path: '.' },
isClientInitiated: false,
prompt_id: 'prompt-id-1',
},
tool: {} as AnyDeclarativeTool,
invocation: {} as AnyToolInvocation,
response: {
callId: 'call_1',
responseParts: [{ text: 'file1.txt\nfile2.ts' }],
resultDisplay: 'Listed 2 files',
error: undefined,
errorType: undefined,
contentLength: undefined,
},
});
const scope = await SubAgentScope.create(
@@ -635,13 +649,25 @@ describe('subagent.ts', () => {
]),
);
// Mock the tool execution failure.
// Mock the tool execution failure - must return CompletedToolCall
vi.mocked(executeToolCall).mockResolvedValue({
callId: 'call_fail',
responseParts: [{ text: 'ERROR: Tool failed catastrophically' }], // This should be sent to the model
resultDisplay: 'Tool failed catastrophically',
error: new Error('Failure'),
errorType: ToolErrorType.INVALID_TOOL_PARAMS,
status: 'error',
request: {
callId: 'call_fail',
name: 'failing_tool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-id-fail',
},
tool: {} as AnyDeclarativeTool,
response: {
callId: 'call_fail',
responseParts: [{ text: 'ERROR: Tool failed catastrophically' }], // This should be sent to the model
resultDisplay: 'Tool failed catastrophically',
error: new Error('Failure'),
errorType: ToolErrorType.INVALID_TOOL_PARAMS,
contentLength: undefined,
},
});
const scope = await SubAgentScope.create(

View File

@@ -580,11 +580,13 @@ export class SubAgentScope {
error: undefined,
};
} else {
toolResponse = await executeToolCall(
this.runtimeContext,
requestInfo,
abortController.signal,
);
toolResponse = (
await executeToolCall(
this.runtimeContext,
requestInfo,
abortController.signal,
)
).response;
}
if (toolResponse.error) {