mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-26 14:01:14 -07:00
refactor(cli,core): foundational layout, identity management, and type safety
This commit establishes the structural foundation and required infrastructure to support the upcoming compact tool output changes. It includes identity management improvements, layout fixes, and type-safety enhancements that stand independently. 1. Identity & History Management: - useHistoryManager: Ensure strictly increasing IDs for history items, even with identical timestamps. - acpClient: Introduced callIdCounter to prevent callId collisions during rapid execution. - MainContent: Implemented negative ID logic for pending items to ensure stable React keys and prevent collisions. - historyUtils: New file containing extracted history logic (isToolExecuting, getAllToolCalls) hoisted from AppContainer. 2. UI Infrastructure & Layout: - test-utils/render: Modernized renderWithProviders and removed legacy props. - AskUserDialog: Fixed layout, scroll visibility, and alignment issues. - toolLayoutUtils: Increased TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT for better spacing. - ShellToolMessage/ToolGroupMessage: Updated line-count expectations and snapshots to align with layout changes. 3. IDE & Diffing Improvements: - ToolActionsContext: Refactored IdeClient initialization to fix a race condition and potential memory leak. - edit/diffOptions: Improved accuracy of diff stat derivation, ensuring "full content" stats are available for the model. - coreToolScheduler: Automatically derive diff stats from patches if missing. - state-manager: Ensure diffStat preservation for rejected tool calls. 4. Type Safety & Constants: - types/tools: Added foundational types like DiffStat, FileDiff, and StructuredToolResult. - Type Guards: Added guards for isFileDiff, isTodoList, isAnsiOutput, and hasSummary. - CodeColorizer: Added function overloads to gracefully handle null language detection. - tool-names: Introduced DISPLAY_NAME constants for consistent tool labeling. This commit passes all workspace tests and builds successfully. Feature-specific logic for compact output is excluded.
This commit is contained in:
@@ -16,7 +16,7 @@ import { vi } from 'vitest';
|
||||
import stripAnsi from 'strip-ansi';
|
||||
import type React from 'react';
|
||||
import { act, useState } from 'react';
|
||||
import type { LoadedSettings } from '../config/settings.js';
|
||||
import { LoadedSettings } from '../config/settings.js';
|
||||
import { KeypressProvider } from '../ui/contexts/KeypressContext.js';
|
||||
import { SettingsContext } from '../ui/contexts/SettingsContext.js';
|
||||
import { ShellFocusContext } from '../ui/contexts/ShellFocusContext.js';
|
||||
@@ -608,16 +608,18 @@ export const renderWithProviders = async (
|
||||
uiState: providedUiState,
|
||||
width,
|
||||
mouseEventsEnabled = false,
|
||||
useAlternateBuffer: explicitUseAlternateBuffer,
|
||||
config,
|
||||
uiActions,
|
||||
persistentState,
|
||||
appState = mockAppState,
|
||||
}: {
|
||||
shellFocus?: boolean;
|
||||
settings?: LoadedSettings;
|
||||
settings?: LoadedSettings | Partial<LoadedSettings['merged']>;
|
||||
uiState?: Partial<UIState>;
|
||||
width?: number;
|
||||
mouseEventsEnabled?: boolean;
|
||||
useAlternateBuffer?: boolean;
|
||||
config?: Config;
|
||||
uiActions?: Partial<UIActions>;
|
||||
persistentState?: {
|
||||
@@ -656,15 +658,34 @@ export const renderWithProviders = async (
|
||||
|
||||
const terminalWidth = width ?? baseState.terminalWidth;
|
||||
|
||||
const finalSettings =
|
||||
settings instanceof LoadedSettings
|
||||
? settings
|
||||
: createMockSettings(settings || {});
|
||||
|
||||
if (!config) {
|
||||
config = await loadCliConfig(
|
||||
settings.merged,
|
||||
finalSettings.merged,
|
||||
'random-session-id',
|
||||
{} as unknown as CliArgs,
|
||||
{} as CliArgs,
|
||||
{ cwd: '/' },
|
||||
);
|
||||
}
|
||||
|
||||
const useAlternateBuffer =
|
||||
explicitUseAlternateBuffer ??
|
||||
finalSettings.merged.ui?.useAlternateBuffer ??
|
||||
false;
|
||||
|
||||
const finalConfig = new Proxy(config, {
|
||||
get(target, prop) {
|
||||
if (prop === 'getUseAlternateBuffer') {
|
||||
return () => useAlternateBuffer;
|
||||
}
|
||||
return Reflect.get(target, prop);
|
||||
},
|
||||
});
|
||||
|
||||
const mainAreaWidth = providedUiState?.mainAreaWidth ?? terminalWidth;
|
||||
|
||||
const finalUiState = {
|
||||
@@ -693,8 +714,8 @@ export const renderWithProviders = async (
|
||||
|
||||
const wrapWithProviders = (comp: React.ReactElement) => (
|
||||
<AppContext.Provider value={appState}>
|
||||
<ConfigContext.Provider value={config}>
|
||||
<SettingsContext.Provider value={settings}>
|
||||
<ConfigContext.Provider value={finalConfig}>
|
||||
<SettingsContext.Provider value={finalSettings}>
|
||||
<UIStateContext.Provider value={finalUiState}>
|
||||
<VimModeProvider>
|
||||
<ShellFocusContext.Provider value={shellFocus}>
|
||||
@@ -705,7 +726,7 @@ export const renderWithProviders = async (
|
||||
<UIActionsContext.Provider value={finalUIActions}>
|
||||
<OverflowProvider>
|
||||
<ToolActionsProvider
|
||||
config={config}
|
||||
config={finalConfig}
|
||||
toolCalls={allToolCalls}
|
||||
>
|
||||
<AskUserActionsProvider
|
||||
@@ -831,7 +852,7 @@ export async function renderHookWithProviders<Result, Props>(
|
||||
wrapper?: React.ComponentType<{ children: React.ReactNode }>;
|
||||
// Options for renderWithProviders
|
||||
shellFocus?: boolean;
|
||||
settings?: LoadedSettings;
|
||||
settings?: LoadedSettings | Partial<LoadedSettings['merged']>;
|
||||
uiState?: Partial<UIState>;
|
||||
width?: number;
|
||||
mouseEventsEnabled?: boolean;
|
||||
|
||||
@@ -88,6 +88,7 @@ export const MainContent = () => {
|
||||
() =>
|
||||
augmentedHistory.map(
|
||||
({ item, isExpandable, isFirstThinking, isFirstAfterThinking }) => (
|
||||
// ({ item, isExpandable, isFirstThinking, /* isFirstAfterThinking */ }) => (
|
||||
<MemoizedHistoryItemDisplay
|
||||
terminalWidth={mainAreaWidth}
|
||||
availableTerminalHeight={
|
||||
|
||||
@@ -71,13 +71,18 @@ describe('ToolActionsContext', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
// Default to a pending promise to avoid unwanted async state updates in tests
|
||||
// that don't specifically test the IdeClient initialization.
|
||||
vi.mocked(IdeClient.getInstance).mockReturnValue(new Promise(() => {}));
|
||||
});
|
||||
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<ToolActionsProvider config={mockConfig} toolCalls={mockToolCalls}>
|
||||
{children}
|
||||
</ToolActionsProvider>
|
||||
);
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => {
|
||||
return (
|
||||
<ToolActionsProvider config={mockConfig} toolCalls={mockToolCalls}>
|
||||
{children}
|
||||
</ToolActionsProvider>
|
||||
);
|
||||
};
|
||||
|
||||
it('publishes to MessageBus for tools with correlationId', async () => {
|
||||
const { result } = await renderHook(() => useToolActions(), { wrapper });
|
||||
|
||||
@@ -5,13 +5,7 @@
|
||||
*/
|
||||
|
||||
import type React from 'react';
|
||||
import {
|
||||
createContext,
|
||||
useContext,
|
||||
useCallback,
|
||||
useState,
|
||||
useEffect,
|
||||
} from 'react';
|
||||
import { createContext, useContext, useCallback, useState, useEffect } from 'react';
|
||||
import {
|
||||
IdeClient,
|
||||
ToolConfirmationOutcome,
|
||||
@@ -52,7 +46,7 @@ interface ToolActionsContextValue {
|
||||
|
||||
const ToolActionsContext = createContext<ToolActionsContextValue | null>(null);
|
||||
|
||||
export const useToolActions = () => {
|
||||
export const useToolActions = (): ToolActionsContextValue => {
|
||||
const context = useContext(ToolActionsContext);
|
||||
if (!context) {
|
||||
throw new Error('useToolActions must be used within a ToolActionsProvider');
|
||||
@@ -77,24 +71,23 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (
|
||||
|
||||
useEffect(() => {
|
||||
let isMounted = true;
|
||||
let activeClient: IdeClient | null = null;
|
||||
|
||||
const handleStatusChange = () => {
|
||||
if (isMounted && activeClient) {
|
||||
setIsDiffingEnabled(activeClient.isDiffingEnabled());
|
||||
}
|
||||
};
|
||||
|
||||
if (config.getIdeMode()) {
|
||||
IdeClient.getInstance()
|
||||
.then((client) => {
|
||||
if (!isMounted) return;
|
||||
activeClient = client;
|
||||
setIdeClient(client);
|
||||
setIsDiffingEnabled(client.isDiffingEnabled());
|
||||
|
||||
const handleStatusChange = () => {
|
||||
if (isMounted) {
|
||||
setIsDiffingEnabled(client.isDiffingEnabled());
|
||||
}
|
||||
};
|
||||
|
||||
client.addStatusChangeListener(handleStatusChange);
|
||||
// Return a cleanup function for the listener
|
||||
return () => {
|
||||
client.removeStatusChangeListener(handleStatusChange);
|
||||
};
|
||||
})
|
||||
.catch((error) => {
|
||||
debugLogger.error('Failed to get IdeClient instance:', error);
|
||||
@@ -102,6 +95,9 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (
|
||||
}
|
||||
return () => {
|
||||
isMounted = false;
|
||||
if (activeClient) {
|
||||
activeClient.removeStatusChangeListener(handleStatusChange);
|
||||
}
|
||||
};
|
||||
}, [config]);
|
||||
|
||||
@@ -164,7 +160,13 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (
|
||||
);
|
||||
|
||||
return (
|
||||
<ToolActionsContext.Provider value={{ confirm, cancel, isDiffingEnabled }}>
|
||||
<ToolActionsContext.Provider
|
||||
value={{
|
||||
confirm,
|
||||
cancel,
|
||||
isDiffingEnabled,
|
||||
}}
|
||||
>
|
||||
{children}
|
||||
</ToolActionsContext.Provider>
|
||||
);
|
||||
|
||||
@@ -93,16 +93,19 @@ export function mapCoreStatusToDisplayStatus(
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* --- TYPE GUARDS ---
|
||||
*/
|
||||
|
||||
|
||||
export const isTodoList = (res: unknown): res is { todos: unknown[] } =>
|
||||
typeof res === 'object' && res !== null && 'todos' in res;
|
||||
|
||||
export const isAnsiOutput = (res: unknown): res is AnsiOutput =>
|
||||
Array.isArray(res) && (res.length === 0 || Array.isArray(res[0]));
|
||||
|
||||
|
||||
export interface ToolCallEvent {
|
||||
type: 'tool_call';
|
||||
status: CoreToolCallStatus;
|
||||
|
||||
@@ -920,7 +920,6 @@ export const isListResult = (
|
||||
res: unknown,
|
||||
): res is ListDirectoryResult | ReadManyFilesResult =>
|
||||
isStructuredToolResult(res) && 'files' in res && Array.isArray(res.files);
|
||||
|
||||
export type ToolResultDisplay =
|
||||
| string
|
||||
| FileDiff
|
||||
|
||||
Reference in New Issue
Block a user