diff --git a/packages/cli/src/ui/hooks/toolMapping.ts b/packages/cli/src/ui/hooks/toolMapping.ts index afb8b34755..0ee950c6bf 100644 --- a/packages/cli/src/ui/hooks/toolMapping.ts +++ b/packages/cli/src/ui/hooks/toolMapping.ts @@ -42,10 +42,9 @@ export function mapToDisplay( if (call.status === CoreToolCallStatus.Error) { description = JSON.stringify(call.request.args); } else { - description = - typeof call.invocation.getDisplayTitle === 'function' - ? call.invocation.getDisplayTitle() - : call.invocation.getDescription(); + description = call.invocation.getDisplayTitle + ? call.invocation.getDisplayTitle() + : call.invocation.getDescription(); renderOutputAsMarkdown = call.tool.isOutputMarkdown; } diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index b62df0d11b..3f1acad836 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -1026,10 +1026,9 @@ export class LocalAgentExecutor { const invocation = tool.build(args); // Prefer getDisplayTitle if it differs, otherwise fallback to getDescription. // This ensures the timeline ("Shell (Replace)") matches the confirmation dialog. - description = - typeof invocation.getDisplayTitle === 'function' - ? invocation.getDisplayTitle() - : invocation.getDescription(); + description = invocation.getDisplayTitle + ? invocation.getDisplayTitle() + : invocation.getDescription(); } } catch { // Ignore errors during formatting for activity emission diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 3fb966ecf0..312622d481 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -30,9 +30,12 @@ import { isGemini2Model, supportsModernFeatures, } from '../config/models.js'; -import { hasCycleInSchema } from '../tools/tools.js'; +import { hasCycleInSchema, hasDisplayTitle } from '../tools/tools.js'; import type { StructuredError } from './turn.js'; -import type { CompletedToolCall } from '../scheduler/types.js'; +import { + type CompletedToolCall, + isInvocationCall, +} from '../scheduler/types.js'; import { logContentRetry, logContentRetryFailure, @@ -1031,11 +1034,10 @@ export class GeminiChat { : undefined; let description: string | undefined = undefined; - if ('invocation' in call && call.invocation) { - description = - typeof call.invocation.getDisplayTitle === 'function' - ? call.invocation.getDisplayTitle() - : call.invocation.getDescription(); + if (isInvocationCall(call)) { + description = hasDisplayTitle(call.invocation) + ? call.invocation.getDisplayTitle() + : call.invocation.getDescription(); } return { diff --git a/packages/core/src/scheduler/types.ts b/packages/core/src/scheduler/types.ts index 170aab67ca..073d98bef9 100644 --- a/packages/core/src/scheduler/types.ts +++ b/packages/core/src/scheduler/types.ts @@ -5,13 +5,14 @@ */ import type { Part } from '@google/genai'; -import type { - AnyDeclarativeTool, - AnyToolInvocation, - ToolCallConfirmationDetails, - ToolConfirmationOutcome, - ToolResultDisplay, - ToolLiveOutput, +import { + isAnyToolInvocation, + type AnyDeclarativeTool, + type AnyToolInvocation, + type ToolCallConfirmationDetails, + type ToolConfirmationOutcome, + type ToolResultDisplay, + type ToolLiveOutput, } from '../tools/tools.js'; import type { ToolErrorType } from '../tools/tool-error.js'; import type { SerializableConfirmationDetails } from '../confirmation-bus/types.js'; @@ -195,6 +196,12 @@ export type CompletedToolCall = | CancelledToolCall | ErroredToolCall; +export function isInvocationCall( + call: CompletedToolCall, +): call is CompletedToolCall & { invocation: AnyToolInvocation } { + return 'invocation' in call && isAnyToolInvocation(call.invocation); +} + export type ConfirmHandler = ( toolCall: WaitingToolCall, ) => Promise; diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index a19520f0e1..231a1eef59 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -139,6 +139,7 @@ describe('ShellTool', () => { getShellBackgroundCompletionBehavior: vi.fn().mockReturnValue('silent'), getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true), getSandboxEnabled: vi.fn().mockReturnValue(false), + getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), sanitizationConfig: {}, sandboxManager: new NoopSandboxManager(), } as unknown as Config; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 29ec9fb9ba..fddb1e57a9 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -44,7 +44,6 @@ import { parseCommandDetails, hasRedirection, inferFileOperation, - FileOperationType, } from '../utils/shell-utils.js'; import { logFileOperation } from '../telemetry/loggers.js'; import { FileOperationEvent } from '../telemetry/types.js'; @@ -143,13 +142,13 @@ export class ShellToolInvocation extends BaseToolInvocation< const inferred = inferFileOperation(this.params.command); if (inferred) { switch (inferred.type) { - case FileOperationType.SEARCH: + case 'search': return `Shell (Search)`; - case FileOperationType.EDIT: + case 'edit': return `Shell (Replace)`; - case FileOperationType.WRITE: + case 'write': return `Shell (Write File)`; - case FileOperationType.READ: + case 'read': return `Shell (Read File)`; default: break; @@ -225,11 +224,7 @@ export class ShellToolInvocation extends BaseToolInvocation< const command = stripShellWrapper(this.params.command); const inferred = inferFileOperation(this.params.command); - if ( - inferred && - (inferred.type === FileOperationType.EDIT || - inferred.type === FileOperationType.WRITE) - ) { + if (inferred && (inferred.type === 'edit' || inferred.type === 'write')) { const filePath = path.resolve( this.context.config.getTargetDir(), inferred.filePath, @@ -242,23 +237,24 @@ export class ShellToolInvocation extends BaseToolInvocation< } let newContent: string | undefined; - if ( - inferred.type === FileOperationType.EDIT && - originalContent !== null - ) { - if (typeof inferred.metadata?.['sedExpression'] === 'string') { + if (inferred.type === 'edit' && originalContent !== null) { + if ( + inferred.metadata?.type === 'edit' && + inferred.metadata.sedExpression + ) { newContent = this.simulateSed( originalContent, - inferred.metadata['sedExpression'], + inferred.metadata.sedExpression, ); } else if ( - typeof inferred.metadata?.['oldString'] === 'string' && - typeof inferred.metadata?.['newString'] === 'string' + inferred.metadata?.type === 'edit' && + inferred.metadata.oldString && + inferred.metadata.newString ) { newContent = this.simulatePsReplace( originalContent, - inferred.metadata['oldString'], - inferred.metadata['newString'], + inferred.metadata.oldString, + inferred.metadata.newString, ); } } @@ -501,19 +497,19 @@ export class ShellToolInvocation extends BaseToolInvocation< let operation: FileOperation | undefined; let canonicalToolName = SHELL_TOOL_NAME; switch (inferred.type) { - case FileOperationType.SEARCH: + case 'search': operation = FileOperation.READ; canonicalToolName = GREP_TOOL_NAME; break; - case FileOperationType.READ: + case 'read': operation = FileOperation.READ; canonicalToolName = READ_FILE_TOOL_NAME; break; - case FileOperationType.EDIT: + case 'edit': operation = FileOperation.UPDATE; canonicalToolName = EDIT_TOOL_NAME; break; - case FileOperationType.WRITE: + case 'write': operation = FileOperation.UPDATE; canonicalToolName = WRITE_FILE_TOOL_NAME; break; diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 23e88b608b..072a9c951f 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -392,6 +392,32 @@ export abstract class BaseToolInvocation< */ export type AnyToolInvocation = ToolInvocation; +/** + * Type guard to check if an object is an AnyToolInvocation. + */ +export function isAnyToolInvocation( + invocation: unknown, +): invocation is AnyToolInvocation { + if (typeof invocation !== 'object' || invocation === null) { + return false; + } + + return ( + 'execute' in invocation && + typeof invocation.execute === 'function' && + 'getDescription' in invocation && + typeof invocation.getDescription === 'function' && + 'params' in invocation && + isRecord(invocation.params) + ); +} + +export function hasDisplayTitle( + invocation: AnyToolInvocation, +): invocation is AnyToolInvocation & { getDisplayTitle(): string } { + return typeof invocation.getDisplayTitle === 'function'; +} + /** * Interface for a tool builder that validates parameters and creates invocations. */ diff --git a/packages/core/src/utils/shell-utils.test.ts b/packages/core/src/utils/shell-utils.test.ts index 6c4c6c381d..e628e6a932 100644 --- a/packages/core/src/utils/shell-utils.test.ts +++ b/packages/core/src/utils/shell-utils.test.ts @@ -24,7 +24,6 @@ import { hasRedirection, resolveExecutable, inferFileOperation, - FileOperationType, } from './shell-utils.js'; import path from 'node:path'; @@ -611,16 +610,19 @@ describe('inferFileOperation', () => { mockPlatform.mockReturnValue('linux'); const result = inferFileOperation("sed -i 's/foo/bar/g' test.ts"); expect(result).toBeDefined(); - expect(result?.type).toBe(FileOperationType.EDIT); + expect(result?.type).toBe('edit'); expect(result?.filePath).toBe('test.ts'); - expect(result?.metadata?.['sedExpression']).toBe('s/foo/bar/g'); + expect(result?.metadata?.type).toBe('edit'); + if (result?.metadata?.type === 'edit') { + expect(result.metadata.sedExpression).toBe('s/foo/bar/g'); + } }); it('should infer echo redirection as a WRITE operation', () => { mockPlatform.mockReturnValue('linux'); const result = inferFileOperation("echo 'hello' > hello.txt"); expect(result).toBeDefined(); - expect(result?.type).toBe(FileOperationType.WRITE); + expect(result?.type).toBe('write'); expect(result?.filePath).toBe('hello.txt'); }); @@ -628,9 +630,12 @@ describe('inferFileOperation', () => { mockPlatform.mockReturnValue('linux'); const result = inferFileOperation("grep 'pattern' file.txt"); expect(result).toBeDefined(); - expect(result?.type).toBe(FileOperationType.SEARCH); + expect(result?.type).toBe('search'); expect(result?.filePath).toBe('file.txt'); - expect(result?.metadata?.['pattern']).toBe('pattern'); + expect(result?.metadata?.type).toBe('search'); + if (result?.metadata?.type === 'search') { + expect(result.metadata.pattern).toBe('pattern'); + } }); it('should infer PowerShell -replace as an EDIT operation', () => { @@ -639,9 +644,12 @@ describe('inferFileOperation', () => { "(Get-Content file.txt) -replace 'a', 'b' | Set-Content file.txt", ); expect(result).toBeDefined(); - expect(result?.type).toBe(FileOperationType.EDIT); + expect(result?.type).toBe('edit'); expect(result?.filePath).toBe('file.txt'); - expect(result?.metadata?.['oldString']).toBe('a'); - expect(result?.metadata?.['newString']).toBe('b'); + expect(result?.metadata?.type).toBe('edit'); + if (result?.metadata?.type === 'edit') { + expect(result.metadata.oldString).toBe('a'); + expect(result.metadata.newString).toBe('b'); + } }); }); diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 466a98b7e0..b1a5ec806b 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -623,18 +623,39 @@ export function getShellConfiguration(): ShellConfiguration { */ export const isWindows = () => os.platform() === 'win32'; -export enum FileOperationType { - SEARCH = 'SEARCH', - EDIT = 'EDIT', - WRITE = 'WRITE', - READ = 'READ', -} +export type FileOperationType = 'search' | 'edit' | 'write' | 'read'; + +export type SearchFileMetadata = { + type: 'search'; + pattern: string; +}; + +export type EditFileMetadata = { + type: 'edit'; + sedExpression?: string; + oldString?: string; + newString?: string; +}; + +export type WriteFileMetadata = { + type: 'write'; +}; + +export type ReadFileMetadata = { + type: 'read'; +}; + +export type InferredFileMetadata = + | SearchFileMetadata + | EditFileMetadata + | WriteFileMetadata + | ReadFileMetadata; export interface InferredFileOperation { type: FileOperationType; filePath: string; /** Inferred details (e.g., the sed expression or replacement string) */ - metadata?: Record; + metadata?: InferredFileMetadata; } /** @@ -783,9 +804,9 @@ export function inferFileOperation( ); if (sedMatch) { return { - type: FileOperationType.EDIT, + type: 'edit', filePath: sedMatch[2].trim(), - metadata: { sedExpression: sedMatch[1] }, + metadata: { type: 'edit', sedExpression: sedMatch[1] }, }; } @@ -796,7 +817,7 @@ export function inferFileOperation( ); if (redirectionMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: redirectionMatch[1].trim(), }; } @@ -807,7 +828,7 @@ export function inferFileOperation( ); if (heredocMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: heredocMatch[1].trim(), }; } @@ -818,7 +839,7 @@ export function inferFileOperation( ); if (cpMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: cpMatch[2].trim(), }; } @@ -829,7 +850,7 @@ export function inferFileOperation( ); if (mvMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: mvMatch[2].trim(), }; } @@ -840,7 +861,7 @@ export function inferFileOperation( ); if (teeMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: teeMatch[1].trim(), }; } @@ -851,9 +872,9 @@ export function inferFileOperation( ); if (grepMatch) { return { - type: FileOperationType.SEARCH, + type: 'search', filePath: grepMatch[2].trim(), - metadata: { pattern: grepMatch[1] }, + metadata: { type: 'search', pattern: grepMatch[1] }, }; } @@ -863,7 +884,7 @@ export function inferFileOperation( ); if (readMatch) { return { - type: FileOperationType.READ, + type: 'read', filePath: readMatch[1].trim(), }; } @@ -874,9 +895,10 @@ export function inferFileOperation( ); if (psReplaceMatch) { return { - type: FileOperationType.EDIT, + type: 'edit', filePath: psReplaceMatch[1].trim(), metadata: { + type: 'edit', oldString: psReplaceMatch[2], newString: psReplaceMatch[3], }, @@ -889,7 +911,7 @@ export function inferFileOperation( ); if (psSetContentMatch) { return { - type: FileOperationType.WRITE, + type: 'write', filePath: psSetContentMatch[1].trim(), }; } @@ -900,9 +922,9 @@ export function inferFileOperation( ); if (psSearchMatch) { return { - type: FileOperationType.SEARCH, + type: 'search', filePath: psSearchMatch[1].trim(), - metadata: { pattern: psSearchMatch[2] }, + metadata: { type: 'search', pattern: psSearchMatch[2] }, }; } @@ -912,7 +934,7 @@ export function inferFileOperation( ); if (psReadMatch) { return { - type: FileOperationType.READ, + type: 'read', filePath: psReadMatch[1].trim(), }; }