refactor(cli): migrate core tools to native ToolDisplay property and fix UI rendering (#25186)

This commit is contained in:
Michael Bleigh
2026-05-06 14:23:26 -07:00
committed by GitHub
parent 4a10751b49
commit 90304b279c
33 changed files with 1033 additions and 57 deletions
+10 -8
View File
@@ -236,6 +236,7 @@ export function translateEvent(
requestId: event.value.callId,
name: event.value.name,
args: event.value.args,
display: event.value.display,
}),
);
break;
@@ -243,13 +244,15 @@ export function translateEvent(
case GeminiEventType.ToolCallResponse: {
ensureStreamStart(state, out);
const data = buildToolResponseData(event.value);
const display: ToolDisplay | undefined = event.value.resultDisplay
? {
result: toolResultDisplayToDisplayContent(
event.value.resultDisplay,
),
}
: undefined;
const display: ToolDisplay | undefined =
event.value.display ??
(event.value.resultDisplay
? {
result: toolResultDisplayToDisplayContent(
event.value.resultDisplay,
),
}
: undefined);
out.push(
makeEvent('tool_response', state, {
requestId: event.value.callId,
@@ -279,7 +282,6 @@ export function translateEvent(
((x: never) => {
throw new Error(`Unhandled event type: ${JSON.stringify(x)}`);
})(event);
break;
}
return out;
@@ -102,7 +102,10 @@ function makeCompletedToolCall(
response: {
callId,
responseParts: [{ text: responseText }],
resultDisplay: undefined,
resultDisplay: responseText,
display: {
result: { type: 'text', text: responseText },
},
error: undefined,
errorType: undefined,
},
@@ -426,6 +429,12 @@ describe('LegacyAgentSession', () => {
(e): e is AgentEvent<'tool_response'> => e.type === 'tool_response',
);
expect(toolResp?.name).toBe('read_file');
expect(toolResp?.display).toEqual(
expect.objectContaining({
name: 'read_file',
result: { type: 'text', text: 'file contents' },
}),
);
expect(toolResp?.content).toEqual([
{ type: 'text', text: 'file contents' },
]);
@@ -266,6 +266,7 @@ export class LegacyAgentProtocol implements AgentProtocol {
invocation: 'invocation' in tc ? tc.invocation : undefined,
resultDisplay: response.resultDisplay,
displayName: 'tool' in tc ? tc.tool?.displayName : undefined,
display: response.display,
});
const data = buildToolResponseData(response);
@@ -21,18 +21,21 @@ export function populateToolDisplay({
invocation,
resultDisplay,
displayName,
display: prevDisplay,
}: {
name: string;
invocation?: ToolInvocation<object, ToolResult>;
resultDisplay?: ToolResultDisplay;
displayName?: string;
display?: ToolDisplay;
}): ToolDisplay {
const display: ToolDisplay = {
name: displayName || name,
description: invocation?.getDescription?.(),
...prevDisplay,
};
if (resultDisplay) {
if (resultDisplay !== undefined && display.result === undefined) {
display.result = toolResultDisplayToDisplayContent(resultDisplay);
}
@@ -91,7 +94,7 @@ export function renderDisplayDiff(diff: DisplayDiff): string {
* Useful for fallback displays or non-interactive environments.
*/
export function displayContentToString(
display: DisplayContent | undefined,
display: DisplayContent | undefined | null,
): string | undefined {
if (!display) {
return undefined;
+39 -3
View File
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import type { AnsiOutput } from '../utils/terminalSerializer.js';
import type { Kind } from '../tools/tools.js';
export type WithMeta = { _meta?: Record<string, unknown> };
@@ -182,13 +183,48 @@ export type DisplayDiff = {
beforeText: string;
afterText: string;
};
export type DisplayContent = DisplayText | DisplayDiff;
export type DisplayTerminal = {
type: 'terminal';
pid?: string;
exitCode?: number;
ansi?: AnsiOutput;
};
export type DisplayAgent = {
type: 'agent';
threadId: string;
};
export type DisplayContent =
| DisplayText
| DisplayDiff
| DisplayTerminal
| DisplayAgent;
export type ToolDisplayFormat =
/**
* Displays as compact when user has enabled compact tools, box otherwise.
* This is the default format if none is selected.
**/
| 'auto'
/** Always display this tool in compact format. */
| 'compact'
/** Always display this tool in full box format. */
| 'box'
/** Hide this tool from the event history. */
| 'hidden'
/** Display this tool as a message-like notice. */
| 'notice';
export interface ToolDisplay {
/** A display name for the tool. */
name?: string;
/** A short description of what the tool is doing. */
description?: string;
resultSummary?: string;
result?: DisplayContent;
/** A short, one-line summary of the tool's results. */
resultSummary?: string | null;
result?: DisplayContent | null;
/** A tool may specify its preferred display format. */
format?: ToolDisplayFormat;
}
export interface ToolRequest {
+8 -2
View File
@@ -3694,11 +3694,17 @@ export class Config implements McpContext, AgentLoopContext {
}
getAgentSessionNoninteractiveEnabled(): boolean {
return this.agentSessionNoninteractiveEnabled;
return (
process.env['GEMINI_CLI_EXP_AGENT'] === 'true' ||
this.agentSessionNoninteractiveEnabled
);
}
getAgentSessionInteractiveEnabled(): boolean {
return this.agentSessionInteractiveEnabled;
return (
process.env['GEMINI_CLI_EXP_AGENT'] === 'true' ||
this.agentSessionInteractiveEnabled
);
}
/**
+4
View File
@@ -284,6 +284,10 @@ export class GeminiChat {
);
}
get loopContext(): AgentLoopContext {
return this.context;
}
async initialize(
resumedSessionData?: ResumedSessionData,
kind: 'main' | 'subagent' = 'main',
+10
View File
@@ -49,6 +49,11 @@ describe('Turn', () => {
getHistory: typeof mockGetHistory;
maybeIncludeSchemaDepthContext: typeof mockMaybeIncludeSchemaDepthContext;
context: { config: { isContextManagementEnabled: () => boolean } };
loopContext?: {
toolRegistry: {
getTool: (name: string) => unknown;
};
};
};
let mockChatInstance: MockedChatInstance;
@@ -63,6 +68,11 @@ describe('Turn', () => {
isContextManagementEnabled: () => false,
},
},
loopContext: {
toolRegistry: {
getTool: vi.fn().mockReturnValue(undefined),
},
},
};
turn = new Turn(mockChatInstance as unknown as GeminiChat, 'prompt-id-1');
mockGetHistory.mockReturnValue([]);
+25 -1
View File
@@ -29,6 +29,7 @@ import { parseThought, type ThoughtSummary } from '../utils/thoughtUtils.js';
import type { ModelConfigKey } from '../services/modelConfigService.js';
import { getCitations } from '../utils/generateContentResponseUtilities.js';
import { LlmRole } from '../telemetry/types.js';
import { populateToolDisplay } from '../agent/tool-display-utils.js';
import {
type ToolCallRequestInfo,
@@ -408,17 +409,40 @@ export class Turn {
traceId?: string,
): ServerGeminiStreamEvent | null {
const name = fnCall.name || 'undefined_tool_name';
const args = fnCall.args || {};
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
const args = (fnCall.args as Record<string, unknown>) || {};
const callId =
fnCall.id ??
(this.chat.context.config.isContextManagementEnabled()
? `synth_${this.prompt_id}_${Date.now()}_${this.callCounter++}`
: `${name}_${Date.now()}_${this.callCounter++}`);
const tool = this.chat.loopContext.toolRegistry.getTool(name);
let display;
if (tool) {
let invocation;
try {
invocation = tool.build(args);
} catch {
// Ignore build errors for request display purposes
}
display = populateToolDisplay({
name,
invocation,
displayName: tool.displayName,
});
// Fallback to static description if invocation failed or didn't provide one
if (!display.description) {
display.description = tool.description;
}
}
const toolCallRequest: ToolCallRequestInfo = {
callId,
name,
args,
display,
isClientInitiated: false,
prompt_id: this.prompt_id,
traceId,
@@ -407,7 +407,7 @@ describe('Scheduler (Orchestrator)', () => {
expect.arrayContaining([
expect.objectContaining({
status: CoreToolCallStatus.Validating,
request: req1,
request: expect.objectContaining(req1),
tool: mockTool,
invocation: mockInvocation,
schedulerId: ROOT_SCHEDULER_ID,
+11
View File
@@ -36,6 +36,7 @@ import { getToolSuggestion } from '../utils/tool-utils.js';
import { runInDevTraceSpan } from '../telemetry/trace.js';
import { logToolCall } from '../telemetry/loggers.js';
import { ToolCallEvent } from '../telemetry/types.js';
import { populateToolDisplay } from '../agent/tool-display-utils.js';
import type { EditorType } from '../utils/editor.js';
import {
MessageBusType,
@@ -381,6 +382,16 @@ export class Scheduler {
() => {
try {
const invocation = tool.build(request.args);
if (!request.display) {
request.display = populateToolDisplay({
name: tool.name,
invocation,
displayName: tool.displayName,
});
if (!request.display.description) {
request.display.description = tool.description;
}
}
return {
status: CoreToolCallStatus.Validating,
request,
+7 -1
View File
@@ -23,6 +23,7 @@ import type {
ToolConfirmationOutcome,
ToolResultDisplay,
AnyToolInvocation,
ToolDisplay,
ToolCallConfirmationDetails,
AnyDeclarativeTool,
} from '../tools/tools.js';
@@ -172,10 +173,15 @@ export class SchedulerStateManager {
const call = this.activeCalls.get(callId);
if (!call || call.status === CoreToolCallStatus.Error) return;
const display: ToolDisplay = call.request.display
? { ...call.request.display }
: { name: call.request.name };
display.description = newInvocation.getDescription();
this.activeCalls.set(
callId,
this.patchCall(call, {
request: { ...call.request, args: newArgs },
request: { ...call.request, args: newArgs, display },
invocation: newInvocation,
}),
);
@@ -12,6 +12,7 @@ import {
type ToolCallRequestInfo,
type ToolCallResponseInfo,
type ToolResult,
type ToolDisplay,
type Config,
type AgentLoopContext,
type ToolLiveOutput,
@@ -160,6 +161,7 @@ export class ToolExecutor {
toolResult.error.type,
displayText,
toolResult.tailToolCallRequest,
toolResult.display,
);
}
} catch (executionError: unknown) {
@@ -350,6 +352,7 @@ export class ToolExecutor {
response: {
callId: call.request.callId,
responseParts,
display: toolResult?.display,
resultDisplay: toolResult?.returnDisplay,
error: undefined,
errorType: undefined,
@@ -386,6 +389,7 @@ export class ToolExecutor {
const successResponse: ToolCallResponseInfo = {
callId,
responseParts: response,
display: toolResult.display,
resultDisplay: toolResult.returnDisplay,
error: undefined,
errorType: undefined,
@@ -420,12 +424,14 @@ export class ToolExecutor {
errorType?: ToolErrorType,
returnDisplay?: string,
tailToolCallRequest?: { name: string; args: Record<string, unknown> },
display?: ToolDisplay,
): ErroredToolCall {
const response = this.createErrorResponse(
call.request,
error,
errorType,
returnDisplay,
display,
);
const startTime = 'startTime' in call ? call.startTime : undefined;
@@ -447,11 +453,13 @@ export class ToolExecutor {
error: Error,
errorType: ToolErrorType | undefined,
returnDisplay?: string,
display?: ToolDisplay,
): ToolCallResponseInfo {
const displayText = returnDisplay ?? error.message;
return {
callId: request.callId,
error,
display,
responseParts: [
{
functionResponse: {
+5
View File
@@ -12,6 +12,7 @@ import type {
ToolConfirmationOutcome,
ToolResultDisplay,
ToolLiveOutput,
ToolDisplay,
} from '../tools/tools.js';
import type { ToolErrorType } from '../tools/tool-error.js';
import type { SerializableConfirmationDetails } from '../confirmation-bus/types.js';
@@ -36,6 +37,8 @@ export interface ToolCallRequestInfo {
callId: string;
name: string;
args: Record<string, unknown>;
/** Tool-controlled display information. */
display?: ToolDisplay;
/**
* The original name and arguments of the tool requested by the model.
* This is used for tail calls to ensure the final response and log retains
@@ -56,6 +59,8 @@ export interface ToolCallRequestInfo {
export interface ToolCallResponseInfo {
callId: string;
responseParts: Part[];
/** Tool-controlled display information. */
display?: ToolDisplay;
resultDisplay: ToolResultDisplay | undefined;
error: Error | undefined;
errorType: ToolErrorType | undefined;
+11
View File
@@ -720,6 +720,17 @@ function doIt() {
});
expect(result.llmContent).toMatch(/Successfully modified file/);
expect(result.display).toEqual(
expect.objectContaining({
name: 'Edit',
resultSummary: expect.stringContaining('added'),
result: expect.objectContaining({
type: 'diff',
beforeText: initialContent,
afterText: newContent,
}),
}),
);
expect(fs.readFileSync(filePath, 'utf8')).toBe(newContent);
const display = result.returnDisplay as FileDiff;
expect(display.fileDiff).toMatch(initialContent);
+23
View File
@@ -22,6 +22,7 @@ import {
type ToolResultDisplay,
type PolicyUpdateOptions,
type ExecuteOptions,
type FileDiff,
} from './tools.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
@@ -431,6 +432,12 @@ export function isEditToolParams(args: unknown): args is EditToolParams {
);
}
function fileDiffToSummary(diff: FileDiff, editData: CalculatedEdit) {
return diff.diffStat
? `${diff.diffStat.model_added_lines} added, ${diff.diffStat.model_removed_lines} removed`
: `${editData.occurrences} replacements`;
}
interface CalculatedEdit {
currentContent: string | null;
newContent: string;
@@ -995,8 +1002,24 @@ ${snippet}`);
llmContent = appendJitContext(llmContent, jitContext);
}
const resultSummary =
typeof displayResult === 'string'
? displayResult
: fileDiffToSummary(displayResult, editData);
return {
llmContent,
display: {
name: this._toolDisplayName,
description: this.getDescription(),
resultSummary,
result: {
type: 'diff',
path: this.resolvedPath,
beforeText: editData.currentContent ?? '',
afterText: editData.newContent,
},
},
returnDisplay: displayResult,
};
} catch (error) {
+13 -1
View File
@@ -284,12 +284,24 @@ class GrepToolInvocation extends BaseToolInvocation<
searchLocationDescription = `in path "${searchDirDisplay}"`;
}
return await formatGrepResults(
const result = await formatGrepResults(
allMatches,
this.params,
searchLocationDescription,
totalMaxMatches,
);
return {
...result,
display: {
name: this._toolDisplayName,
description: this.getDescription(),
resultSummary: result.returnDisplay.summary,
result: {
type: 'text',
text: result.llmContent.split('\n---\n').slice(1).join('\n---\n'),
},
},
};
} catch (error) {
debugLogger.warn(`Error during GrepLogic execution: ${error}`);
const errorMessage = getErrorMessage(error);
+5
View File
@@ -284,6 +284,11 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
return {
llmContent: resultMessage,
display: {
name: LS_DISPLAY_NAME,
description: this.getDescription(),
resultSummary: displayMessage,
},
returnDisplay: {
summary: displayMessage,
files: entries.map(
+24 -8
View File
@@ -237,10 +237,18 @@ describe('ReadFileTool', () => {
const params: ReadFileToolParams = { file_path: 'textfile.txt' };
const invocation = tool.build(params);
expect(await invocation.execute({ abortSignal })).toEqual({
llmContent: fileContent,
returnDisplay: '',
});
const result = await invocation.execute({ abortSignal });
expect(result).toEqual(
expect.objectContaining({
llmContent: fileContent,
returnDisplay: '',
display: expect.objectContaining({
name: 'ReadFile',
description: expect.stringContaining('textfile.txt'),
resultSummary: '1 lines',
}),
}),
);
});
it('should return error if file does not exist', async () => {
@@ -267,10 +275,18 @@ describe('ReadFileTool', () => {
const params: ReadFileToolParams = { file_path: filePath };
const invocation = tool.build(params);
expect(await invocation.execute({ abortSignal })).toEqual({
llmContent: fileContent,
returnDisplay: '',
});
const result = await invocation.execute({ abortSignal });
expect(result).toEqual(
expect.objectContaining({
llmContent: fileContent,
returnDisplay: '',
display: expect.objectContaining({
name: 'ReadFile',
description: expect.stringContaining('textfile.txt'),
resultSummary: '1 lines',
}),
}),
);
});
it('should return error if path is a directory', async () => {
+12
View File
@@ -186,8 +186,20 @@ ${result.llmContent}`;
}
}
const displayResultSummary = result.isTruncated
? `${result.linesShown![0]}-${result.linesShown![1]} of ${result.originalLineCount}`
: lines !== undefined
? `${lines} lines`
: undefined;
return {
llmContent,
display: {
name: READ_FILE_DISPLAY_NAME,
description: this.getDescription(),
resultSummary: displayResultSummary,
result: { type: 'text', text: result.returnDisplay || '' },
},
returnDisplay: result.returnDisplay || '',
};
}
+13 -1
View File
@@ -301,12 +301,24 @@ class GrepToolInvocation extends BaseToolInvocation<
const searchLocationDescription = `in path "${searchDirDisplay}"`;
return await formatGrepResults(
const result = await formatGrepResults(
allMatches,
this.params,
searchLocationDescription,
totalMaxMatches,
);
return {
...result,
display: {
name: this._toolDisplayName,
description: this.getDescription(),
resultSummary: result.returnDisplay.summary,
result: {
type: 'text',
text: result.llmContent.split('\n---\n').slice(1).join('\n---\n'),
},
},
};
} catch (error) {
debugLogger.warn(`Error during GrepLogic execution: ${error}`);
const errorMessage = getErrorMessage(error);
+7
View File
@@ -504,6 +504,13 @@ EOF`;
const result = await promise;
expect(result.llmContent).toContain('Error: wrapped command failed');
expect(result.llmContent).not.toContain('pgrep');
expect(result.display).toEqual(
expect.objectContaining({
name: 'Shell',
description: 'user-command',
resultSummary: 'Exit Code: 1',
}),
);
});
it('should return a SHELL_EXECUTE_ERROR for a command failure', async () => {
+16
View File
@@ -942,8 +942,24 @@ export class ShellToolInvocation extends BaseToolInvocation<
};
}
const displayResultSummary = result.backgrounded
? `PID: ${result.pid}`
: result.exitCode !== null && result.exitCode !== 0
? `Exit Code: ${result.exitCode}`
: undefined;
return {
llmContent,
display: {
name: 'Shell',
description: this.getDescription(),
resultSummary: displayResultSummary,
result:
typeof returnDisplay === 'string'
? { type: 'text', text: returnDisplay }
: // TODO: Add support for terminal display type (AnsiOutput)
undefined,
},
returnDisplay,
data,
...executionError,
+7
View File
@@ -740,6 +740,10 @@ export function isTool(obj: unknown): obj is AnyDeclarativeTool {
}
export interface ToolResult {
/**
* Tool-controlled display information.
*/
display?: ToolDisplay;
/**
* Content meant to be included in LLM history.
* This should represent the factual outcome of the tool execution.
@@ -1084,6 +1088,9 @@ export type ToolCallConfirmationDetails =
| ToolAskUserConfirmationDetails
| ToolExitPlanModeConfirmationDetails;
import type { ToolDisplay } from '../agent/types.js';
export type { ToolDisplay };
export enum ToolConfirmationOutcome {
ProceedOnce = 'proceed_once',
ProceedAlways = 'proceed_always',
+5
View File
@@ -93,6 +93,11 @@ class UpdateTopicInvocation extends BaseToolInvocation<
return {
llmContent,
display: {
format: 'notice',
name: title || UPDATE_TOPIC_DISPLAY_NAME,
description: this.getDescription(),
},
returnDisplay,
};
}
@@ -678,6 +678,16 @@ describe('WriteFileTool', () => {
expect(result.llmContent).toMatch(
/Successfully created and wrote to new file/,
);
expect(result.display).toEqual(
expect.objectContaining({
name: 'WriteFile',
resultSummary: expect.stringContaining('added'),
result: expect.objectContaining({
type: 'diff',
afterText: content,
}),
}),
);
expect(fs.existsSync(filePath)).toBe(true);
const writtenContent = await fsService.readTextFile(filePath);
expect(writtenContent).toBe(content);
+13
View File
@@ -430,6 +430,19 @@ class WriteFileToolInvocation extends BaseToolInvocation<
return {
llmContent,
display: {
name: WRITE_FILE_DISPLAY_NAME,
description: this.getDescription(),
resultSummary: diffStat
? `${diffStat.model_added_lines} added, ${diffStat.model_removed_lines} removed`
: 'Written',
result: {
type: 'diff',
path: this.resolvedPath,
beforeText: correctedContentResult.originalContent ?? '',
afterText: correctedContentResult.correctedContent,
},
},
returnDisplay: displayResult,
};
} catch (error) {