Skip to content

Fix bottom border color #19266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jacob314 merged 3 commits into from
Feb 18, 2026
Merged

Fix bottom border color #19266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25baa15
Fix bottom border color
jacob314 Feb 17, 2026
f927439
Checkpoint of code review fixes
jacob314 Feb 18, 2026
bfd02c5
Refactor to avoid memoizing params that shouldn't be memoized.
jacob314 Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions packages/cli/src/ui/App.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,18 @@ describe('App', () => {

const stateWithConfirmingTool = {
...mockUIState,
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
pendingGeminiHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
pendingHistoryItems: [
{
type: 'tool_group',
tools: toolCalls,
},
],
pendingGeminiHistoryItems: [
{
type: 'tool_group',
tools: toolCalls,
},
],
} as UIState;

const configWithExperiment = makeFakeConfig();
Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ export const AlternateBufferQuittingDisplay = () => {
terminalWidth={uiState.mainAreaWidth}
item={{ ...item, id: 0 }}
isPending={true}
activeShellPtyId={uiState.activePtyId}
embeddedShellFocused={uiState.embeddedShellFocused}
/>
))}
{showPromptedTool && (
Expand Down
8 changes: 1 addition & 7 deletions packages/cli/src/ui/components/HistoryItemDisplay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ interface HistoryItemDisplayProps {
terminalWidth: number;
isPending: boolean;
commands?: readonly SlashCommand[];
activeShellPtyId?: number | null;
embeddedShellFocused?: boolean;
availableTerminalHeightGemini?: number;
}

Expand All @@ -55,8 +53,6 @@ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({
terminalWidth,
isPending,
commands,
activeShellPtyId,
embeddedShellFocused,
availableTerminalHeightGemini,
}) => {
const settings = useSettings();
Expand Down Expand Up @@ -173,12 +169,10 @@ export const HistoryItemDisplay: React.FC<HistoryItemDisplayProps> = ({
)}
{itemForDisplay.type === 'tool_group' && (
<ToolGroupMessage
item={itemForDisplay}
toolCalls={itemForDisplay.tools}
groupId={itemForDisplay.id}
availableTerminalHeight={availableTerminalHeight}
terminalWidth={terminalWidth}
activeShellPtyId={activeShellPtyId}
embeddedShellFocused={embeddedShellFocused}
borderTop={itemForDisplay.borderTop}
borderBottom={itemForDisplay.borderBottom}
/>
Expand Down
207 changes: 206 additions & 1 deletion packages/cli/src/ui/components/MainContent.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { renderWithProviders } from '../../test-utils/render.js';
import { waitFor } from '../../test-utils/async.js';
import { MainContent } from './MainContent.js';
import { getToolGroupBorderAppearance } from '../utils/borderStyles.js';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { Box, Text } from 'ink';
import { act, useState, type JSX } from 'react';
Expand All @@ -18,6 +19,7 @@ import {
type UIState,
} from '../contexts/UIStateContext.js';
import { CoreToolCallStatus } from '@google/gemini-cli-core';
import { type IndividualToolCallDisplay } from '../types.js';

// Mock dependencies
vi.mock('../contexts/SettingsContext.js', async () => {
Expand Down Expand Up @@ -76,6 +78,209 @@ vi.mock('./shared/ScrollableList.js', () => ({
SCROLL_TO_ITEM_END: 0,
}));

import { theme } from '../semantic-colors.js';
import { type BackgroundShell } from '../hooks/shellReducer.js';

describe('getToolGroupBorderAppearance', () => {
const mockBackgroundShells = new Map<number, BackgroundShell>();
const activeShellPtyId = 123;

it('returns default empty values for non-tool_group items', () => {
const item = { type: 'user' as const, text: 'Hello', id: 1 };
const result = getToolGroupBorderAppearance(
item,
null,
false,
[],
mockBackgroundShells,
);
expect(result).toEqual({ borderColor: '', borderDimColor: false });
});

it('inspects only the last pending tool_group item if current has no tools', () => {
const item = { type: 'tool_group' as const, tools: [], id: 1 };
const pendingItems = [
{
type: 'tool_group' as const,
tools: [
{
callId: '1',
name: 'some_tool',
description: '',
status: CoreToolCallStatus.Executing,
ptyId: undefined,
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
},
{
type: 'tool_group' as const,
tools: [
{
callId: '2',
name: 'other_tool',
description: '',
status: CoreToolCallStatus.Success,
ptyId: undefined,
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
},
];

// Only the last item (Success) should be inspected, so hasPending = false.
// The previous item was Executing (pending) but it shouldn't be counted.
const result = getToolGroupBorderAppearance(
item,
null,
false,
pendingItems,
mockBackgroundShells,
);
expect(result).toEqual({
borderColor: theme.border.default,
borderDimColor: false,
});
});

it('returns default border for completed normal tools', () => {
const item = {
type: 'tool_group' as const,
tools: [
{
callId: '1',
name: 'some_tool',
description: '',
status: CoreToolCallStatus.Success,
ptyId: undefined,
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
id: 1,
};
const result = getToolGroupBorderAppearance(
item,
null,
false,
[],
mockBackgroundShells,
);
expect(result).toEqual({
borderColor: theme.border.default,
borderDimColor: false,
});
});

it('returns warning border for pending normal tools', () => {
const item = {
type: 'tool_group' as const,
tools: [
{
callId: '1',
name: 'some_tool',
description: '',
status: CoreToolCallStatus.Executing,
ptyId: undefined,
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
id: 1,
};
const result = getToolGroupBorderAppearance(
item,
null,
false,
[],
mockBackgroundShells,
);
expect(result).toEqual({
borderColor: theme.status.warning,
borderDimColor: true,
});
});

it('returns symbol border for executing shell commands', () => {
const item = {
type: 'tool_group' as const,
tools: [
{
callId: '1',
name: SHELL_COMMAND_NAME,
description: '',
status: CoreToolCallStatus.Executing,
ptyId: activeShellPtyId,
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
id: 1,
};
// While executing shell commands, it's dim false, border symbol
const result = getToolGroupBorderAppearance(
item,
activeShellPtyId,
true,
[],
mockBackgroundShells,
);
expect(result).toEqual({
borderColor: theme.ui.symbol,
borderDimColor: false,
});
});

it('returns symbol border and dims color for background executing shell command when another shell is active', () => {
const item = {
type: 'tool_group' as const,
tools: [
{
callId: '1',
name: SHELL_COMMAND_NAME,
description: '',
status: CoreToolCallStatus.Executing,
ptyId: 456, // Different ptyId, not active
resultDisplay: undefined,
confirmationDetails: undefined,
} as IndividualToolCallDisplay,
],
id: 1,
};
const result = getToolGroupBorderAppearance(
item,
activeShellPtyId,
false,
[],
mockBackgroundShells,
);
expect(result).toEqual({
borderColor: theme.ui.symbol,
borderDimColor: true,
});
});

it('handles empty tools with active shell turn (isCurrentlyInShellTurn)', () => {
const item = { type: 'tool_group' as const, tools: [], id: 1 };

// active shell turn
const result = getToolGroupBorderAppearance(
item,
activeShellPtyId,
true,
[],
mockBackgroundShells,
);
// Since there are no tools to inspect, it falls back to empty pending, but isCurrentlyInShellTurn=true
// so it counts as pending shell.
expect(result.borderColor).toEqual(theme.ui.symbol);
// It shouldn't be dim because there are no tools to say it isEmbeddedShellFocused = false
Copy link
Copy Markdown
Collaborator

@galz10 galz10 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions isCurrentlyInShellTurn=true but doesn't explicitly verify the borderDimColor expectation, which would be helpful for clarity.

Copy link
Copy Markdown
Contributor Author

@jacob314 jacob314 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

expect(result.borderDimColor).toBe(false);
});
});

describe('MainContent', () => {
const defaultMockUiState = {
history: [
Expand Down Expand Up @@ -258,7 +463,7 @@ describe('MainContent', () => {
history: [],
pendingHistoryItems: [
{
type: 'tool_group' as const,
type: 'tool_group',
id: 1,
tools: [
{
Expand Down
4 changes: 0 additions & 4 deletions packages/cli/src/ui/components/MainContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ export const MainContent = () => {
terminalWidth={mainAreaWidth}
item={{ ...item, id: 0 }}
isPending={true}
activeShellPtyId={uiState.activePtyId}
embeddedShellFocused={uiState.embeddedShellFocused}
/>
))}
{showConfirmationQueue && confirmingTool && (
Expand All @@ -103,8 +101,6 @@ export const MainContent = () => {
isAlternateBuffer,
availableTerminalHeight,
mainAreaWidth,
uiState.activePtyId,
uiState.embeddedShellFocused,
showConfirmationQueue,
confirmingTool,
],
Expand Down
48 changes: 28 additions & 20 deletions packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,16 @@ describe('<ShellToolMessage />', () => {
CoreToolCallStatus.Executing,
);
updateStatus = setStatus;
return (
<ShellToolMessage
{...baseProps}
status={status}
embeddedShellFocused={true}
activeShellPtyId={1}
ptyId={1}
/>
);
return <ShellToolMessage {...baseProps} status={status} ptyId={1} />;
};

const { lastFrame } = renderWithProviders(<Wrapper />, {
uiActions,
uiState: { streamingState: StreamingState.Idle },
uiState: {
streamingState: StreamingState.Idle,
embeddedShellFocused: true,
activePtyId: 1,
},
});

// Verify it is initially focused
Expand Down Expand Up @@ -143,21 +139,29 @@ describe('<ShellToolMessage />', () => {
'renders in Alternate Buffer mode while focused',
{
status: CoreToolCallStatus.Executing,
embeddedShellFocused: true,
activeShellPtyId: 1,
ptyId: 1,
},
{ useAlternateBuffer: true },
{
useAlternateBuffer: true,
uiState: {
embeddedShellFocused: true,
activePtyId: 1,
},
},
],
[
'renders in Alternate Buffer mode while unfocused',
{
status: CoreToolCallStatus.Executing,
embeddedShellFocused: false,
activeShellPtyId: 1,
ptyId: 1,
},
{ useAlternateBuffer: true },
{
useAlternateBuffer: true,
uiState: {
embeddedShellFocused: false,
activePtyId: 1,
},
},
],
])('%s', async (_, props, options) => {
const { lastFrame } = renderShell(props, options);
Expand Down Expand Up @@ -199,12 +203,16 @@ describe('<ShellToolMessage />', () => {
resultDisplay: LONG_OUTPUT,
renderOutputAsMarkdown: false,
availableTerminalHeight,
activeShellPtyId: 1,
ptyId: focused ? 1 : 2,
ptyId: 1,
status: CoreToolCallStatus.Executing,
embeddedShellFocused: focused,
},
{ useAlternateBuffer: true },
{
useAlternateBuffer: true,
uiState: {
activePtyId: focused ? 1 : 2,
embeddedShellFocused: focused,
},
},
);

await waitFor(() => {
Expand Down
Loading
Loading