fix(ui): resolve rebase conflicts and type errors for ToolDisplay

This commit is contained in:
Michael Bleigh
2026-04-10 20:18:51 -07:00
parent fbc87675f0
commit 43f93c3cde
24 changed files with 276 additions and 38 deletions

View File

@@ -24,6 +24,7 @@ import {
type ToolResultDisplay,
isTodoList,
} from '../../types.js';
import { isCompactTool } from './ToolGroupMessage.js';
import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js';
import { ToolStatusIndicator } from './ToolShared.js';
import { theme } from '../../semantic-colors.js';
@@ -286,6 +287,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
const [isFocused, setIsFocused] = useState(false);
const toggleRef = useRef<DOMElement>(null);
const isActuallyCompact = useMemo(() => isCompactTool(props, true), [props]);
// Unified File Data Extraction (Safely bridge resultDisplay and confirmationDetails)
const diff = useMemo((): FileDiff | undefined => {
@@ -369,11 +371,36 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
}
}
// If we have a display.result or a simple success, use it
// If we have a display.result, use it as the payload
let payload: React.ReactNode;
if (display.result) {
if (display.result.type === 'text') {
const text = display.result.text;
if (text) {
payload = (
<Text color={theme.text.secondary} wrap="truncate-end">
{text}
</Text>
);
}
}
// Step 5 will expand this to handle 'diff' type
}
// Compact tools should elide text payloads by default unless expanded.
if (
isActuallyCompact &&
!isExpanded &&
display.result?.type === 'text' &&
!isAlternateBuffer
) {
payload = undefined;
}
return {
description: descriptionText,
summary: summaryText,
payload: undefined, // Payload rendering will be updated in Step 5
payload,
};
}
@@ -439,6 +466,8 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
originalDescription,
isAlternateBuffer,
display,
isActuallyCompact,
isExpanded,
]);
const { description, summary } = viewParts;
@@ -476,6 +505,10 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
}, [diff, isExpanded, isAlternateBuffer, terminalWidth, settings, status]);
const showPayload = useMemo(() => {
// If we are using the new display protocol and it's a compact tool,
// hide the payload by default unless expanded.
if (display && isActuallyCompact && !isExpanded) return false;
const policy = !isAlternateBuffer || !diff || isExpanded;
if (!policy) return false;
@@ -495,6 +528,8 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
diffLines.length,
viewParts.payload,
outputFile,
isActuallyCompact,
display,
]);
const keyExtractor = (_item: React.ReactNode, index: number) =>
@@ -505,7 +540,16 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
return (
<Box flexDirection="column">
<Box marginLeft={2} flexDirection="row" flexWrap="wrap">
<Box
marginLeft={2}
flexDirection="row"
flexWrap="wrap"
ref={
isActuallyCompact || (isAlternateBuffer && diff)
? toggleRef
: undefined
}
>
<Box flexDirection="row" flexShrink={1}>
<ToolStatusIndicator status={status} name={name} />
<Box maxWidth={25} flexShrink={0} flexGrow={0}>
@@ -519,12 +563,7 @@ export const DenseToolMessage: React.FC<DenseToolMessageProps> = (props) => {
</Box>
{summary && (
<Box
key="tool-summary"
ref={isAlternateBuffer && diff ? toggleRef : undefined}
marginLeft={1}
flexGrow={0}
>
<Box key="tool-summary" marginLeft={1} flexGrow={0}>
{summary}
</Box>
)}

View File

@@ -87,6 +87,11 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
resultDisplay,
);
const effectiveResultDisplay =
display?.resultSummary && !display.result
? display.resultSummary
: resultDisplay;
return (
// It is crucial we don't replace this <> with a Box because otherwise the
// sticky header inside it would be sticky to that box rather than to the
@@ -139,7 +144,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
/>
)}
<ToolResultDisplay
resultDisplay={resultDisplay}
resultDisplay={effectiveResultDisplay}
availableTerminalHeight={availableTerminalHeight}
terminalWidth={terminalWidth}
renderOutputAsMarkdown={renderOutputAsMarkdown}

View File

@@ -8,6 +8,7 @@ import {
type ToolCall,
type SerializableConfirmationDetails,
type ToolResultDisplay,
type ToolDisplay,
debugLogger,
CoreToolCallStatus,
type SubagentActivityItem,
@@ -35,10 +36,17 @@ export function mapToDisplay(
borderBottom?: boolean;
borderColor?: string;
borderDimColor?: boolean;
isAgentSessionInteractive?: boolean;
} = {},
): HistoryItemToolGroup {
const toolCalls = Array.isArray(toolOrTools) ? toolOrTools : [toolOrTools];
const { borderTop, borderBottom, borderColor, borderDimColor } = options;
const {
borderTop,
borderBottom,
borderColor,
borderDimColor,
isAgentSessionInteractive,
} = options;
const toolDisplays = toolCalls.map((call): IndividualToolCallDisplay => {
let description: string;
@@ -63,6 +71,7 @@ export function mapToDisplay(
};
let resultDisplay: ToolResultDisplay | undefined = undefined;
let display: ToolDisplay | undefined = undefined;
let confirmationDetails: SerializableConfirmationDetails | undefined =
undefined;
let outputFile: string | undefined = undefined;
@@ -75,11 +84,17 @@ export function mapToDisplay(
switch (call.status) {
case CoreToolCallStatus.Success:
resultDisplay = call.response.resultDisplay;
if (isAgentSessionInteractive) {
display = call.response.display;
}
outputFile = call.response.outputFile;
break;
case CoreToolCallStatus.Error:
case CoreToolCallStatus.Cancelled:
resultDisplay = call.response.resultDisplay;
if (isAgentSessionInteractive) {
display = call.response.display;
}
break;
case CoreToolCallStatus.AwaitingApproval:
correlationId = call.correlationId;
@@ -112,6 +127,7 @@ export function mapToDisplay(
status: call.status,
isClientInitiated: !!call.request.isClientInitiated,
kind: call.tool?.kind,
display,
resultDisplay,
confirmationDetails,
outputFile,

View File

@@ -224,9 +224,9 @@ export const useAgentStream = ({
else if (evtStatus === 'success')
status = CoreToolCallStatus.Success;
const display = event.display?.result;
const liveOutput =
displayContentToString(display) ?? tc.resultDisplay;
displayContentToString(event.display?.result) ??
tc.resultDisplay;
const progressMessage =
legacyState?.progressMessage ?? tc.progressMessage;
const progress = legacyState?.progress ?? tc.progress;
@@ -237,6 +237,7 @@ export const useAgentStream = ({
return {
...tc,
name: event.display?.name ?? tc.name,
status,
display: event.display
? { ...tc.display, ...event.display }
@@ -259,12 +260,13 @@ export const useAgentStream = ({
const legacyState = event._meta?.legacyState;
const outputFile = legacyState?.outputFile;
const display = event.display?.result;
const display = event.display;
const resultDisplay =
displayContentToString(display) ?? tc.resultDisplay;
displayContentToString(display?.result) ?? tc.resultDisplay;
return {
...tc,
name: display?.name ?? tc.name,
status: event.isError
? CoreToolCallStatus.Error
: CoreToolCallStatus.Success,

View File

@@ -243,13 +243,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,

View File

@@ -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,
},
@@ -427,6 +430,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' },
]);

View File

@@ -267,6 +267,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);

View File

@@ -21,13 +21,16 @@ export function populateToolDisplay({
invocation,
resultDisplay,
displayName,
display: prevDisplay,
}: {
name: string;
invocation?: ToolInvocation<object, ToolResult>;
resultDisplay?: ToolResultDisplay;
displayName?: string;
display?: ToolDisplay;
}): ToolDisplay {
const display: ToolDisplay = {
...prevDisplay,
name: displayName || name,
description: invocation?.getDescription?.(),
};
@@ -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;

View File

@@ -187,8 +187,8 @@ export type DisplayContent = DisplayText | DisplayDiff;
export interface ToolDisplay {
name?: string;
description?: string;
resultSummary?: string;
result?: DisplayContent;
resultSummary?: string | null;
result?: DisplayContent | null;
}
export interface ToolRequest {

View File

@@ -3422,11 +3422,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
);
}
/**

View File

@@ -12,6 +12,7 @@ import {
type ToolCallRequestInfo,
type ToolCallResponseInfo,
type ToolResult,
type ToolDisplay,
type Config,
type AgentLoopContext,
type ToolLiveOutput,
@@ -159,6 +160,7 @@ export class ToolExecutor {
toolResult.error.type,
displayText,
toolResult.tailToolCallRequest,
toolResult.display,
);
}
} catch (executionError: unknown) {
@@ -349,6 +351,7 @@ export class ToolExecutor {
response: {
callId: call.request.callId,
responseParts,
display: toolResult?.display,
resultDisplay: toolResult?.returnDisplay,
error: undefined,
errorType: undefined,
@@ -385,6 +388,7 @@ export class ToolExecutor {
const successResponse: ToolCallResponseInfo = {
callId,
responseParts: response,
display: toolResult.display,
resultDisplay: toolResult.returnDisplay,
error: undefined,
errorType: undefined,
@@ -419,12 +423,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;
@@ -446,11 +452,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: {

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';
@@ -56,6 +57,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;

View File

@@ -719,6 +719,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);

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';
@@ -430,6 +431,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;
@@ -984,8 +991,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) {

View File

@@ -284,12 +284,21 @@ 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: null,
},
};
} catch (error) {
debugLogger.warn(`Error during GrepLogic execution: ${error}`);
const errorMessage = getErrorMessage(error);

View File

@@ -284,6 +284,19 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
return {
llmContent: resultMessage,
display: {
name: LS_DISPLAY_NAME,
description: this.getDescription(),
resultSummary: displayMessage,
result: {
type: 'text',
text: entries
.map(
(entry) => `${entry.isDirectory ? '[DIR] ' : ''}${entry.name}`,
)
.join('\n'),
},
},
returnDisplay: {
summary: displayMessage,
files: entries.map(

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 () => {

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 || '',
};
}

View File

@@ -326,12 +326,21 @@ 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: null,
},
};
} catch (error) {
debugLogger.warn(`Error during GrepLogic execution: ${error}`);
const errorMessage = getErrorMessage(error);

View File

@@ -480,6 +480,12 @@ describe('ShellTool', () => {
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: 'user-command',
resultSummary: 'Exit Code: 1',
}),
);
});
it('should return a SHELL_EXECUTE_ERROR for a command failure', async () => {

View File

@@ -925,8 +925,23 @@ 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: this.getDisplayTitle(),
description: this.getDescription(),
resultSummary: displayResultSummary,
result:
typeof returnDisplay === 'string'
? { type: 'text', text: returnDisplay }
: undefined,
},
returnDisplay,
data,
...executionError,

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',

View File

@@ -677,6 +677,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);

View File

@@ -420,6 +420,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) {