mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-22 11:04:42 -07:00
refactor(cli,core): foundational layout, identity management, and type safety (#23286)
This commit is contained in:
@@ -8,6 +8,7 @@ import { type FunctionCall } from '@google/genai';
|
||||
import type {
|
||||
ToolConfirmationOutcome,
|
||||
ToolConfirmationPayload,
|
||||
DiffStat,
|
||||
} from '../tools/tools.js';
|
||||
import type { ToolCall } from '../scheduler/types.js';
|
||||
|
||||
@@ -94,6 +95,7 @@ export type SerializableConfirmationDetails =
|
||||
originalContent: string | null;
|
||||
newContent: string;
|
||||
isModifying?: boolean;
|
||||
diffStat?: DiffStat;
|
||||
}
|
||||
| {
|
||||
type: 'exec';
|
||||
|
||||
@@ -22,6 +22,7 @@ import {
|
||||
ToolConfirmationOutcome,
|
||||
type AnyDeclarativeTool,
|
||||
type AnyToolInvocation,
|
||||
type FileDiff,
|
||||
} from '../tools/tools.js';
|
||||
import { MessageBusType } from '../confirmation-bus/types.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
@@ -359,7 +360,7 @@ describe('SchedulerStateManager', () => {
|
||||
expect(active.confirmationDetails).toEqual(details);
|
||||
});
|
||||
|
||||
it('should preserve diff when cancelling an edit tool call', () => {
|
||||
it('should preserve diff and derive stats when cancelling an edit tool call', () => {
|
||||
const call = createValidatingCall();
|
||||
stateManager.enqueue([call]);
|
||||
stateManager.dequeue();
|
||||
@@ -369,9 +370,9 @@ describe('SchedulerStateManager', () => {
|
||||
title: 'Edit',
|
||||
fileName: 'test.txt',
|
||||
filePath: '/path/to/test.txt',
|
||||
fileDiff: 'diff',
|
||||
originalContent: 'old',
|
||||
newContent: 'new',
|
||||
fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line',
|
||||
originalContent: 'old line',
|
||||
newContent: 'new line',
|
||||
onConfirm: vi.fn(),
|
||||
};
|
||||
|
||||
@@ -389,13 +390,14 @@ describe('SchedulerStateManager', () => {
|
||||
|
||||
const completed = stateManager.completedBatch[0] as CancelledToolCall;
|
||||
expect(completed.status).toBe(CoreToolCallStatus.Cancelled);
|
||||
expect(completed.response.resultDisplay).toEqual({
|
||||
fileDiff: 'diff',
|
||||
fileName: 'test.txt',
|
||||
filePath: '/path/to/test.txt',
|
||||
originalContent: 'old',
|
||||
newContent: 'new',
|
||||
});
|
||||
const result = completed.response.resultDisplay as FileDiff;
|
||||
expect(result.fileDiff).toBe(details.fileDiff);
|
||||
expect(result.diffStat).toEqual(
|
||||
expect.objectContaining({
|
||||
model_added_lines: 1,
|
||||
model_removed_lines: 1,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should ignore status updates for non-existent callIds', () => {
|
||||
|
||||
@@ -32,6 +32,7 @@ import {
|
||||
type SerializableConfirmationDetails,
|
||||
} from '../confirmation-bus/types.js';
|
||||
import { isToolCallResponseInfo } from '../utils/tool-utils.js';
|
||||
import { getDiffStatFromPatch } from '../tools/diffOptions.js';
|
||||
|
||||
/**
|
||||
* Handler for terminal tool calls.
|
||||
@@ -473,6 +474,8 @@ export class SchedulerStateManager {
|
||||
filePath: details.filePath,
|
||||
originalContent: details.originalContent,
|
||||
newContent: details.newContent,
|
||||
// Derive stats from the patch if they aren't already present
|
||||
diffStat: details.diffStat ?? getDiffStatFromPatch(details.fileDiff),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,3 +76,39 @@ export function getDiffStat(
|
||||
user_removed_chars: userStats.removedChars,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts line and character stats from a unified diff patch string.
|
||||
* This is useful for reconstructing stats for rejected or errored operations
|
||||
* where the full strings may no longer be easily accessible.
|
||||
*/
|
||||
export function getDiffStatFromPatch(patch: string): DiffStat {
|
||||
let addedLines = 0;
|
||||
let removedLines = 0;
|
||||
let addedChars = 0;
|
||||
let removedChars = 0;
|
||||
|
||||
const lines = patch.split('\n');
|
||||
for (const line of lines) {
|
||||
// Only count lines that are additions or removals,
|
||||
// excluding the diff headers (--- and +++) and metadata (\)
|
||||
if (line.startsWith('+') && !line.startsWith('+++')) {
|
||||
addedLines++;
|
||||
addedChars += line.length - 1;
|
||||
} else if (line.startsWith('-') && !line.startsWith('---')) {
|
||||
removedLines++;
|
||||
removedChars += line.length - 1;
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
model_added_lines: addedLines,
|
||||
model_removed_lines: removedLines,
|
||||
model_added_chars: addedChars,
|
||||
model_removed_chars: removedChars,
|
||||
user_added_lines: 0,
|
||||
user_removed_lines: 0,
|
||||
user_added_chars: 0,
|
||||
user_removed_chars: 0,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -900,11 +900,36 @@ class EditToolInvocation
|
||||
DEFAULT_DIFF_OPTIONS,
|
||||
);
|
||||
|
||||
// Determine the full content as originally proposed by the AI to ensure accurate diff stats.
|
||||
let fullAiProposedContent = editData.newContent;
|
||||
if (
|
||||
this.params.modified_by_user &&
|
||||
this.params.ai_proposed_content !== undefined
|
||||
) {
|
||||
try {
|
||||
const aiReplacement = await calculateReplacement(this.config, {
|
||||
params: {
|
||||
...this.params,
|
||||
new_string: this.params.ai_proposed_content,
|
||||
},
|
||||
currentContent: editData.currentContent ?? '',
|
||||
abortSignal: signal,
|
||||
});
|
||||
fullAiProposedContent = aiReplacement.newContent;
|
||||
} catch (error) {
|
||||
const errorMsg =
|
||||
error instanceof Error ? error.message : String(error);
|
||||
debugLogger.log(`AI replacement fallback: ${errorMsg}`);
|
||||
// Fallback to newContent if speculative calculation fails
|
||||
fullAiProposedContent = editData.newContent;
|
||||
}
|
||||
}
|
||||
|
||||
const diffStat = getDiffStat(
|
||||
fileName,
|
||||
editData.currentContent ?? '',
|
||||
fullAiProposedContent,
|
||||
editData.newContent,
|
||||
this.params.new_string,
|
||||
);
|
||||
displayResult = {
|
||||
fileDiff,
|
||||
|
||||
@@ -150,8 +150,6 @@ export {
|
||||
SKILL_PARAM_NAME,
|
||||
};
|
||||
|
||||
export const LS_TOOL_NAME_LEGACY = 'list_directory'; // Just to be safe if anything used the old exported name directly
|
||||
|
||||
export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]);
|
||||
|
||||
/**
|
||||
@@ -182,6 +180,11 @@ export const EDIT_DISPLAY_NAME = 'Edit';
|
||||
export const ASK_USER_DISPLAY_NAME = 'Ask User';
|
||||
export const READ_FILE_DISPLAY_NAME = 'ReadFile';
|
||||
export const GLOB_DISPLAY_NAME = 'FindFiles';
|
||||
export const LS_DISPLAY_NAME = 'ReadFolder';
|
||||
export const GREP_DISPLAY_NAME = 'SearchText';
|
||||
export const WEB_SEARCH_DISPLAY_NAME = 'GoogleSearch';
|
||||
export const WEB_FETCH_DISPLAY_NAME = 'WebFetch';
|
||||
export const READ_MANY_FILES_DISPLAY_NAME = 'ReadManyFiles';
|
||||
|
||||
/**
|
||||
* Mapping of legacy tool names to their current names.
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
import type { FunctionDeclaration, PartListUnion } from '@google/genai';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import type { GrepMatch } from './grep-utils.js';
|
||||
import type { DiffUpdateResult } from '../ide/ide-client.js';
|
||||
import type { ShellExecutionConfig } from '../services/shellExecutionService.js';
|
||||
import { SchemaValidator } from '../utils/schemaValidator.js';
|
||||
@@ -859,6 +860,51 @@ export interface TodoList {
|
||||
|
||||
export type ToolLiveOutput = string | AnsiOutput | SubagentProgress;
|
||||
|
||||
export interface StructuredToolResult {
|
||||
summary: string;
|
||||
}
|
||||
|
||||
export function isStructuredToolResult(
|
||||
obj: unknown,
|
||||
): obj is StructuredToolResult {
|
||||
return (
|
||||
typeof obj === 'object' &&
|
||||
obj !== null &&
|
||||
'summary' in obj &&
|
||||
typeof obj.summary === 'string'
|
||||
);
|
||||
}
|
||||
|
||||
export const hasSummary = (res: unknown): res is { summary: string } =>
|
||||
isStructuredToolResult(res);
|
||||
|
||||
export interface GrepResult extends StructuredToolResult {
|
||||
matches: GrepMatch[];
|
||||
payload?: string;
|
||||
}
|
||||
|
||||
export interface ListDirectoryResult extends StructuredToolResult {
|
||||
files: string[];
|
||||
payload?: string;
|
||||
}
|
||||
|
||||
export interface ReadManyFilesResult extends StructuredToolResult {
|
||||
files: string[];
|
||||
skipped?: Array<{ path: string; reason: string }>;
|
||||
include?: string[];
|
||||
excludes?: string[];
|
||||
targetDir?: string;
|
||||
payload?: string;
|
||||
}
|
||||
|
||||
export const isGrepResult = (res: unknown): res is GrepResult =>
|
||||
isStructuredToolResult(res) && 'matches' in res && Array.isArray(res.matches);
|
||||
|
||||
export const isListResult = (
|
||||
res: unknown,
|
||||
): res is ListDirectoryResult | ReadManyFilesResult =>
|
||||
isStructuredToolResult(res) && 'files' in res && Array.isArray(res.files);
|
||||
|
||||
export type ToolResultDisplay =
|
||||
| string
|
||||
| FileDiff
|
||||
@@ -888,6 +934,13 @@ export interface FileDiff {
|
||||
isNewFile?: boolean;
|
||||
}
|
||||
|
||||
export const isFileDiff = (res: unknown): res is FileDiff =>
|
||||
typeof res === 'object' &&
|
||||
res !== null &&
|
||||
'fileDiff' in res &&
|
||||
'fileName' in res &&
|
||||
'filePath' in res;
|
||||
|
||||
export interface DiffStat {
|
||||
model_added_lines: number;
|
||||
model_removed_lines: number;
|
||||
@@ -913,6 +966,7 @@ export interface ToolEditConfirmationDetails {
|
||||
originalContent: string | null;
|
||||
newContent: string;
|
||||
isModifying?: boolean;
|
||||
diffStat?: DiffStat;
|
||||
ideConfirmation?: Promise<DiffUpdateResult>;
|
||||
}
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ import {
|
||||
NetworkRetryAttemptEvent,
|
||||
} from '../telemetry/index.js';
|
||||
import { LlmRole } from '../telemetry/llmRole.js';
|
||||
import { WEB_FETCH_TOOL_NAME } from './tool-names.js';
|
||||
import { WEB_FETCH_TOOL_NAME, WEB_FETCH_DISPLAY_NAME } from './tool-names.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { coreEvents } from '../utils/events.js';
|
||||
import { retryWithBackoff, getRetryErrorType } from '../utils/retry.js';
|
||||
@@ -883,7 +883,7 @@ export class WebFetchTool extends BaseDeclarativeTool<
|
||||
) {
|
||||
super(
|
||||
WebFetchTool.Name,
|
||||
'WebFetch',
|
||||
WEB_FETCH_DISPLAY_NAME,
|
||||
WEB_FETCH_DEFINITION.base.description!,
|
||||
Kind.Fetch,
|
||||
WEB_FETCH_DEFINITION.base.parametersJsonSchema,
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import { WEB_SEARCH_TOOL_NAME } from './tool-names.js';
|
||||
import { WEB_SEARCH_TOOL_NAME, WEB_SEARCH_DISPLAY_NAME } from './tool-names.js';
|
||||
import type { GroundingMetadata } from '@google/genai';
|
||||
import {
|
||||
BaseDeclarativeTool,
|
||||
@@ -212,7 +212,7 @@ export class WebSearchTool extends BaseDeclarativeTool<
|
||||
) {
|
||||
super(
|
||||
WebSearchTool.Name,
|
||||
'GoogleSearch',
|
||||
WEB_SEARCH_DISPLAY_NAME,
|
||||
WEB_SEARCH_DEFINITION.base.description!,
|
||||
Kind.Search,
|
||||
WEB_SEARCH_DEFINITION.base.parametersJsonSchema,
|
||||
|
||||
Reference in New Issue
Block a user