mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-01 19:03:42 +00:00
fix(core,cli): handle structured tool display properly and prevent metadata overwrite
This addresses PR feedback by: - Creating a `renderDisplayDiff` utility to render `DisplayDiff` objects. - Creating a `displayContentToString` utility to safely extract text from any `DisplayContent`. - Updating non-interactive CLI to use `displayContentToString` to avoid data loss on non-text output. - Updating `useAgentStream` to use `displayContentToString` to avoid stale UI state for non-text output. - Shallow merging the `display` object in `useAgentStream` rather than replacing it, preventing loss of display metadata.
This commit is contained in:
@@ -37,6 +37,7 @@ import {
|
||||
LegacyAgentSession,
|
||||
ToolErrorType,
|
||||
geminiPartsToContentParts,
|
||||
displayContentToString,
|
||||
debugLogger,
|
||||
} from '@google/gemini-cli-core';
|
||||
|
||||
@@ -470,10 +471,8 @@ export async function runNonInteractive({
|
||||
case 'tool_response': {
|
||||
textOutput.ensureTrailingNewline();
|
||||
if (streamFormatter) {
|
||||
const displayText =
|
||||
event.display?.result?.type === 'text'
|
||||
? event.display.result.text
|
||||
: undefined;
|
||||
const display = event.display?.result;
|
||||
const displayText = displayContentToString(display);
|
||||
const errorMsg = getTextContent(event.content) ?? 'Tool error';
|
||||
streamFormatter.emitEvent({
|
||||
type: JsonStreamEventType.TOOL_RESULT,
|
||||
@@ -493,10 +492,8 @@ export async function runNonInteractive({
|
||||
});
|
||||
}
|
||||
if (event.isError) {
|
||||
const displayText =
|
||||
event.display?.result?.type === 'text'
|
||||
? event.display.result.text
|
||||
: undefined;
|
||||
const display = event.display?.result;
|
||||
const displayText = displayContentToString(display);
|
||||
const errorMsg = getTextContent(event.content) ?? 'Tool error';
|
||||
|
||||
if (event.data?.['errorType'] === ToolErrorType.STOP_EXECUTION) {
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
MessageSenderType,
|
||||
debugLogger,
|
||||
geminiPartsToContentParts,
|
||||
displayContentToString,
|
||||
parseThought,
|
||||
CoreToolCallStatus,
|
||||
type ApprovalMode,
|
||||
@@ -223,10 +224,9 @@ export const useAgentStream = ({
|
||||
else if (evtStatus === 'success')
|
||||
status = CoreToolCallStatus.Success;
|
||||
|
||||
const display = event.display?.result;
|
||||
const liveOutput =
|
||||
event.display?.result?.type === 'text'
|
||||
? event.display.result.text
|
||||
: tc.resultDisplay;
|
||||
displayContentToString(display) ?? tc.resultDisplay;
|
||||
const progressMessage =
|
||||
legacyState?.progressMessage ?? tc.progressMessage;
|
||||
const progress = legacyState?.progress ?? tc.progress;
|
||||
@@ -238,7 +238,9 @@ export const useAgentStream = ({
|
||||
return {
|
||||
...tc,
|
||||
status,
|
||||
display: event.display ?? tc.display,
|
||||
display: event.display
|
||||
? { ...tc.display, ...event.display }
|
||||
: tc.display,
|
||||
resultDisplay: liveOutput,
|
||||
progressMessage,
|
||||
progress,
|
||||
@@ -257,17 +259,18 @@ export const useAgentStream = ({
|
||||
|
||||
const legacyState = event._meta?.legacyState;
|
||||
const outputFile = legacyState?.outputFile;
|
||||
const display = event.display?.result;
|
||||
const resultDisplay =
|
||||
event.display?.result?.type === 'text'
|
||||
? event.display.result.text
|
||||
: tc.resultDisplay;
|
||||
displayContentToString(display) ?? tc.resultDisplay;
|
||||
|
||||
return {
|
||||
...tc,
|
||||
status: event.isError
|
||||
? CoreToolCallStatus.Error
|
||||
: CoreToolCallStatus.Success,
|
||||
display: event.display ?? tc.display,
|
||||
display: event.display
|
||||
? { ...tc.display, ...event.display }
|
||||
: tc.display,
|
||||
resultDisplay,
|
||||
outputFile,
|
||||
};
|
||||
|
||||
@@ -10,7 +10,12 @@ import type {
|
||||
ToolResult,
|
||||
ToolResultDisplay,
|
||||
} from '../tools/tools.js';
|
||||
import { populateToolDisplay } from './tool-display-utils.js';
|
||||
import type { DisplayContent } from './types.js';
|
||||
import {
|
||||
populateToolDisplay,
|
||||
renderDisplayDiff,
|
||||
displayContentToString,
|
||||
} from './tool-display-utils.js';
|
||||
|
||||
describe('tool-display-utils', () => {
|
||||
describe('populateToolDisplay', () => {
|
||||
@@ -68,4 +73,52 @@ describe('tool-display-utils', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderDisplayDiff', () => {
|
||||
it('renders a universal diff', () => {
|
||||
const diff = {
|
||||
type: 'diff' as const,
|
||||
path: 'test.ts',
|
||||
beforeText: 'line 1\nline 2',
|
||||
afterText: 'line 1\nline 2 modified',
|
||||
};
|
||||
const rendered = renderDisplayDiff(diff);
|
||||
expect(rendered).toContain('--- test.ts\tOriginal');
|
||||
expect(rendered).toContain('+++ test.ts\tModified');
|
||||
expect(rendered).toContain('-line 2');
|
||||
expect(rendered).toContain('+line 2 modified');
|
||||
});
|
||||
});
|
||||
|
||||
describe('displayContentToString', () => {
|
||||
it('returns undefined for undefined input', () => {
|
||||
expect(displayContentToString(undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns text for text input', () => {
|
||||
expect(displayContentToString({ type: 'text', text: 'hello' })).toBe(
|
||||
'hello',
|
||||
);
|
||||
});
|
||||
|
||||
it('renders a diff for diff input', () => {
|
||||
const diff = {
|
||||
type: 'diff' as const,
|
||||
path: 'test.ts',
|
||||
beforeText: 'old',
|
||||
afterText: 'new',
|
||||
};
|
||||
const rendered = displayContentToString(diff);
|
||||
expect(rendered).toContain('--- test.ts\tOriginal');
|
||||
expect(rendered).toContain('+++ test.ts\tModified');
|
||||
});
|
||||
|
||||
it('stringifies unknown structured objects', () => {
|
||||
const unknown = {
|
||||
type: 'something_else',
|
||||
data: 123,
|
||||
} as unknown as DisplayContent;
|
||||
expect(displayContentToString(unknown)).toBe(JSON.stringify(unknown));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,12 +4,13 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as Diff from 'diff';
|
||||
import type {
|
||||
ToolInvocation,
|
||||
ToolResult,
|
||||
ToolResultDisplay,
|
||||
} from '../tools/tools.js';
|
||||
import type { ToolDisplay, DisplayContent } from './types.js';
|
||||
import type { ToolDisplay, DisplayContent, DisplayDiff } from './types.js';
|
||||
|
||||
/**
|
||||
* Populates a ToolDisplay object from a tool invocation and its result.
|
||||
@@ -70,3 +71,36 @@ export function toolResultDisplayToDisplayContent(
|
||||
text: JSON.stringify(resultDisplay),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders a universal diff string from a DisplayDiff object.
|
||||
*/
|
||||
export function renderDisplayDiff(diff: DisplayDiff): string {
|
||||
return Diff.createPatch(
|
||||
diff.path || 'file',
|
||||
diff.beforeText,
|
||||
diff.afterText,
|
||||
'Original',
|
||||
'Modified',
|
||||
{ context: 3 },
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts a DisplayContent object into a string representation.
|
||||
* Useful for fallback displays or non-interactive environments.
|
||||
*/
|
||||
export function displayContentToString(
|
||||
display: DisplayContent | undefined,
|
||||
): string | undefined {
|
||||
if (!display) {
|
||||
return undefined;
|
||||
}
|
||||
if (display.type === 'text') {
|
||||
return display.text;
|
||||
}
|
||||
if (display.type === 'diff') {
|
||||
return renderDisplayDiff(display);
|
||||
}
|
||||
return JSON.stringify(display);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user