mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-16 14:53:19 -07:00
fix(cli): ensure consistent tool top-borders during sequences of tool outputs (#24513)
Fixes a visual regression where tool outputs could render without a top
border ("capless") when multiple tools completed at different times or
during transient pending states.
The issue was caused by a synchronization failure between how history
items are "pushed" (from pending to committed items) and how their
borders were determined.
Changes:
- Ensured `pendingToolGroupItems` always uses `borderTop: true` to avoid
transient capless state for executing tools.
- Removed redundant `isFirstInThisPush` flag.
- Added `useGeminiStream.BorderRendering.test.tsx` to permanently cover
sequential tool completion and pending border states.
This commit is contained in:
@@ -0,0 +1,194 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
|
||||
import { act } from 'react';
|
||||
import { useGeminiStream } from './useGeminiStream.js';
|
||||
import { renderHookWithProviders } from '../../test-utils/render.js';
|
||||
import { waitFor } from '../../test-utils/async.js';
|
||||
import {
|
||||
CoreToolCallStatus,
|
||||
Kind,
|
||||
type Config,
|
||||
type AnyToolInvocation,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type {
|
||||
TrackedToolCall,
|
||||
TrackedCompletedToolCall,
|
||||
TrackedExecutingToolCall,
|
||||
} from './useToolScheduler.js';
|
||||
import type { LoadedSettings } from '../../config/settings.js';
|
||||
|
||||
// Mock useToolScheduler
|
||||
const mockScheduleToolCalls = vi.fn();
|
||||
const mockCancelAllToolCalls = vi.fn();
|
||||
const mockMarkToolsAsSubmitted = vi.fn();
|
||||
|
||||
vi.mock('./useToolScheduler.js', () => ({
|
||||
useToolScheduler: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock other hooks
|
||||
vi.mock('./useAlternateBuffer.js', () => ({
|
||||
useAlternateBuffer: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
describe('useGeminiStream Border Logic', () => {
|
||||
let mockAddItem = vi.fn();
|
||||
const mockConfig = {
|
||||
getApprovalMode: vi.fn(() => 'AUTO_EDIT'),
|
||||
getGeminiClient: vi.fn(() => ({
|
||||
getChat: vi.fn(() => ({
|
||||
recordCompletedToolCalls: vi.fn(),
|
||||
})),
|
||||
getCurrentSequenceModel: vi.fn(),
|
||||
})),
|
||||
getModel: vi.fn(() => 'gemini-2.0-flash'),
|
||||
getProjectRoot: vi.fn(),
|
||||
getCheckpointingEnabled: vi.fn(() => false),
|
||||
getContentGeneratorConfig: vi.fn(() => ({ authType: 'none' })),
|
||||
storage: {},
|
||||
} as unknown as Config;
|
||||
|
||||
const mockLoadedSettings = {
|
||||
merged: {
|
||||
ui: {
|
||||
compactToolOutput: true,
|
||||
},
|
||||
},
|
||||
} as LoadedSettings;
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
mockAddItem = vi.fn();
|
||||
const { useToolScheduler } = await import('./useToolScheduler.js');
|
||||
(useToolScheduler as Mock).mockReturnValue([
|
||||
[],
|
||||
mockScheduleToolCalls,
|
||||
mockMarkToolsAsSubmitted,
|
||||
vi.fn(),
|
||||
mockCancelAllToolCalls,
|
||||
0,
|
||||
]);
|
||||
});
|
||||
|
||||
it('should maintain borderTop when standard tools are pushed in separate batches (Fixes #24513)', async () => {
|
||||
let currentToolCalls: TrackedToolCall[] = [];
|
||||
|
||||
const { useToolScheduler } = await import('./useToolScheduler.js');
|
||||
(useToolScheduler as Mock).mockImplementation(() => [
|
||||
currentToolCalls,
|
||||
mockScheduleToolCalls,
|
||||
mockMarkToolsAsSubmitted,
|
||||
vi.fn(),
|
||||
mockCancelAllToolCalls,
|
||||
0,
|
||||
]);
|
||||
|
||||
const tool1: TrackedToolCall = {
|
||||
request: {
|
||||
callId: 'call1',
|
||||
name: 'shell',
|
||||
args: {},
|
||||
prompt_id: 'p1',
|
||||
isClientInitiated: false,
|
||||
},
|
||||
status: CoreToolCallStatus.Success,
|
||||
response: {
|
||||
callId: 'call1',
|
||||
responseParts: [],
|
||||
resultDisplay: 'out1',
|
||||
error: undefined,
|
||||
errorType: undefined,
|
||||
},
|
||||
tool: { kind: Kind.Execute, displayName: 'shell' },
|
||||
invocation: { getDescription: () => 'desc1' } as AnyToolInvocation,
|
||||
responseSubmittedToGemini: false,
|
||||
} as unknown as TrackedCompletedToolCall;
|
||||
|
||||
const tool2Executing: TrackedToolCall = {
|
||||
request: {
|
||||
callId: 'call2',
|
||||
name: 'shell',
|
||||
args: {},
|
||||
prompt_id: 'p1',
|
||||
isClientInitiated: false,
|
||||
},
|
||||
status: CoreToolCallStatus.Executing,
|
||||
tool: { kind: Kind.Execute, displayName: 'shell' },
|
||||
invocation: { getDescription: () => 'desc2' } as AnyToolInvocation,
|
||||
} as unknown as TrackedExecutingToolCall;
|
||||
|
||||
const tool2Success: TrackedToolCall = {
|
||||
...tool2Executing,
|
||||
status: CoreToolCallStatus.Success,
|
||||
response: {
|
||||
callId: 'call2',
|
||||
responseParts: [],
|
||||
resultDisplay: 'out2',
|
||||
error: undefined,
|
||||
errorType: undefined,
|
||||
},
|
||||
responseSubmittedToGemini: false,
|
||||
} as unknown as TrackedCompletedToolCall;
|
||||
|
||||
currentToolCalls = [tool1, tool2Executing];
|
||||
|
||||
const { result, rerender } = await renderHookWithProviders(() =>
|
||||
useGeminiStream(
|
||||
mockConfig.getGeminiClient(),
|
||||
[],
|
||||
mockAddItem,
|
||||
mockConfig,
|
||||
mockLoadedSettings,
|
||||
vi.fn(),
|
||||
vi.fn().mockResolvedValue(false),
|
||||
false,
|
||||
() => undefined,
|
||||
vi.fn(),
|
||||
vi.fn(),
|
||||
false,
|
||||
vi.fn(),
|
||||
vi.fn(),
|
||||
vi.fn(),
|
||||
80,
|
||||
24,
|
||||
),
|
||||
);
|
||||
|
||||
// Initial render should push Tool 1 (since it's success)
|
||||
await waitFor(() => {
|
||||
expect(mockAddItem).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Check pending items - Tool 2 is executing
|
||||
expect(result.current.pendingHistoryItems.length).toBeGreaterThan(0);
|
||||
const pendingItem = result.current.pendingHistoryItems.find(
|
||||
(i) => i.type === 'tool_group',
|
||||
);
|
||||
expect(pendingItem).toBeDefined();
|
||||
// This is expected to FAIL currently (it will be false)
|
||||
expect(pendingItem?.borderTop).toBe(true);
|
||||
|
||||
// Now Tool 2 completes
|
||||
currentToolCalls = [tool1, tool2Success];
|
||||
await act(async () => {
|
||||
rerender();
|
||||
});
|
||||
|
||||
// Second push for Tool 2
|
||||
await waitFor(() => {
|
||||
expect(mockAddItem).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
const call1Item = mockAddItem.mock.calls[0][0];
|
||||
const call2Item = mockAddItem.mock.calls[1][0];
|
||||
|
||||
// Both should have borderTop: true because they were pushed in separate history items
|
||||
expect(call1Item.borderTop).toBe(true);
|
||||
expect(call2Item.borderTop).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -306,10 +306,9 @@ export const useGeminiStream = (
|
||||
const tcIndex = toolCalls.indexOf(firstToolToPush);
|
||||
const prevTool = tcIndex > 0 ? toolCalls[tcIndex - 1] : null;
|
||||
|
||||
let borderTop = isFirstToolInGroupRef.current;
|
||||
if (!borderTop && prevTool) {
|
||||
// If the first tool in this push is non-compact but follows a compact tool,
|
||||
// we must start a new border group.
|
||||
// Start a new box for a new history item
|
||||
let borderTop = true;
|
||||
if (prevTool) {
|
||||
const currentIsCompact = isCompactTool(
|
||||
mapTrackedToolCallsToDisplay(firstToolToPush).tools[0],
|
||||
isCompactModeEnabled,
|
||||
@@ -318,8 +317,10 @@ export const useGeminiStream = (
|
||||
mapTrackedToolCallsToDisplay(prevTool).tools[0],
|
||||
isCompactModeEnabled,
|
||||
);
|
||||
if (!currentIsCompact && prevWasCompact) {
|
||||
borderTop = true;
|
||||
|
||||
// Continue the existing border group for consecutive non-compact tools
|
||||
if (!currentIsCompact && !prevWasCompact) {
|
||||
borderTop = false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -490,7 +491,6 @@ export const useGeminiStream = (
|
||||
|
||||
if (toolsToPush.length > 0) {
|
||||
const newPushed = new Set(pushedToolCallIdsRef.current);
|
||||
const isFirstInThisPush = isFirstToolInGroupRef.current;
|
||||
const isCompactModeEnabled =
|
||||
settings.merged.ui?.compactToolOutput === true;
|
||||
|
||||
@@ -516,19 +516,12 @@ export const useGeminiStream = (
|
||||
|
||||
for (let i = 0; i < groups.length; i++) {
|
||||
const group = groups[i];
|
||||
const isFirstInBatch = i === 0 && isFirstInThisPush;
|
||||
const lastTcInGroup = group[group.length - 1];
|
||||
const tcIndexInBatch = toolCalls.indexOf(lastTcInGroup);
|
||||
const isLastInBatch = tcIndexInBatch === toolCalls.length - 1;
|
||||
const isFirstInBatch = i === 0; // Each new history item starts its own border group
|
||||
const isLastInBatch = i === groups.length - 1; // Each history item closes its own border group
|
||||
|
||||
const nextTcInBatch =
|
||||
tcIndexInBatch < toolCalls.length - 1
|
||||
? toolCalls[tcIndexInBatch + 1]
|
||||
: null;
|
||||
const nextTcInBatch = i < groups.length - 1 ? groups[i + 1][0] : null;
|
||||
const prevTcInBatch =
|
||||
toolCalls.indexOf(group[0]) > 0
|
||||
? toolCalls[toolCalls.indexOf(group[0]) - 1]
|
||||
: null;
|
||||
i > 0 ? groups[i - 1][groups[i - 1].length - 1] : null;
|
||||
|
||||
const historyItem = mapTrackedToolCallsToDisplay(group, {
|
||||
...getToolGroupBorderAppearance(
|
||||
@@ -611,17 +604,9 @@ export const useGeminiStream = (
|
||||
);
|
||||
|
||||
if (remainingTools.length > 0) {
|
||||
// Should we draw a top border? Yes if NO previous tools were drawn,
|
||||
// OR if ALL previously drawn tools were topics (which don't draw top borders).
|
||||
let needsTopBorder = pushedToolCallIds.size === 0;
|
||||
if (!needsTopBorder) {
|
||||
const allPushedWereTopics = toolCalls
|
||||
.filter((tc) => pushedToolCallIds.has(tc.request.callId))
|
||||
.every((tc) => isTopicTool(tc.request.name));
|
||||
if (allPushedWereTopics) {
|
||||
needsTopBorder = true;
|
||||
}
|
||||
}
|
||||
// The pending group always needs a top border because any previously
|
||||
// pushed tool history items have already closed their own border boxes.
|
||||
const needsTopBorder = true;
|
||||
|
||||
items.push(
|
||||
mapTrackedToolCallsToDisplay(remainingTools, {
|
||||
|
||||
Reference in New Issue
Block a user