PR cleanup.

This commit is contained in:
Christian Gunderman
2026-04-13 20:41:55 -07:00
parent 6621bd88c8
commit e2b69dc267
9 changed files with 137 additions and 77 deletions
+3 -4
View File
@@ -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;
}
+3 -4
View File
@@ -1026,10 +1026,9 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
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
+9 -7
View File
@@ -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 {
+14 -7
View File
@@ -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<ToolConfirmationOutcome>;
+1
View File
@@ -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;
+20 -24
View File
@@ -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;
+26
View File
@@ -392,6 +392,32 @@ export abstract class BaseToolInvocation<
*/
export type AnyToolInvocation = ToolInvocation<object, ToolResult>;
/**
* 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.
*/
+17 -9
View File
@@ -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');
}
});
});
+44 -22
View File
@@ -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<string, unknown>;
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(),
};
}