feat(core): Annotate remaining error paths in tools with type. (#6699)
This commit is contained in:
parent
299bf58309
commit
ec41b8db8e
|
@ -9,10 +9,14 @@ import { partListUnionToString } from '../core/geminiRequest.js';
|
|||
import path from 'path';
|
||||
import fs from 'fs/promises';
|
||||
import os from 'os';
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
|
||||
import { Config } from '../config/config.js';
|
||||
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import * as glob from 'glob';
|
||||
|
||||
vi.mock('glob', { spy: true });
|
||||
|
||||
describe('GlobTool', () => {
|
||||
let tempRootDir: string; // This will be the rootDirectory for the GlobTool instance
|
||||
|
@ -203,6 +207,29 @@ describe('GlobTool', () => {
|
|||
path.resolve(tempRootDir, 'older.sortme'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should return a PATH_NOT_IN_WORKSPACE error if path is outside workspace', async () => {
|
||||
// Bypassing validation to test execute method directly
|
||||
vi.spyOn(globTool, 'validateToolParams').mockReturnValue(null);
|
||||
const params: GlobToolParams = { pattern: '*.txt', path: '/etc' };
|
||||
const invocation = globTool.build(params);
|
||||
const result = await invocation.execute(abortSignal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.PATH_NOT_IN_WORKSPACE);
|
||||
expect(result.returnDisplay).toBe('Path is not within workspace');
|
||||
});
|
||||
|
||||
it('should return a GLOB_EXECUTION_ERROR on glob failure', async () => {
|
||||
vi.mocked(glob.glob).mockRejectedValue(new Error('Glob failed'));
|
||||
const params: GlobToolParams = { pattern: '*.txt' };
|
||||
const invocation = globTool.build(params);
|
||||
const result = await invocation.execute(abortSignal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.GLOB_EXECUTION_ERROR);
|
||||
expect(result.llmContent).toContain(
|
||||
'Error during glob search operation: Glob failed',
|
||||
);
|
||||
// Reset glob.
|
||||
vi.mocked(glob.glob).mockReset();
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateToolParams', () => {
|
||||
|
|
|
@ -16,6 +16,7 @@ import {
|
|||
} from './tools.js';
|
||||
import { shortenPath, makeRelative } from '../utils/paths.js';
|
||||
import { Config } from '../config/config.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
// Subset of 'Path' interface provided by 'glob' that we can implement for testing
|
||||
export interface GlobPath {
|
||||
|
@ -115,9 +116,14 @@ class GlobToolInvocation extends BaseToolInvocation<
|
|||
this.params.path,
|
||||
);
|
||||
if (!workspaceContext.isPathWithinWorkspace(searchDirAbsolute)) {
|
||||
const rawError = `Error: Path "${this.params.path}" is not within any workspace directory`;
|
||||
return {
|
||||
llmContent: `Error: Path "${this.params.path}" is not within any workspace directory`,
|
||||
llmContent: rawError,
|
||||
returnDisplay: `Path is not within workspace`,
|
||||
error: {
|
||||
message: rawError,
|
||||
type: ToolErrorType.PATH_NOT_IN_WORKSPACE,
|
||||
},
|
||||
};
|
||||
}
|
||||
searchDirectories = [searchDirAbsolute];
|
||||
|
@ -234,9 +240,14 @@ class GlobToolInvocation extends BaseToolInvocation<
|
|||
const errorMessage =
|
||||
error instanceof Error ? error.message : String(error);
|
||||
console.error(`GlobLogic execute Error: ${errorMessage}`, error);
|
||||
const rawError = `Error during glob search operation: ${errorMessage}`;
|
||||
return {
|
||||
llmContent: `Error during glob search operation: ${errorMessage}`,
|
||||
llmContent: rawError,
|
||||
returnDisplay: `Error: An unexpected error occurred.`,
|
||||
error: {
|
||||
message: rawError,
|
||||
type: ToolErrorType.GLOB_EXECUTION_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
@ -11,6 +11,10 @@ import fs from 'fs/promises';
|
|||
import os from 'os';
|
||||
import { Config } from '../config/config.js';
|
||||
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import * as glob from 'glob';
|
||||
|
||||
vi.mock('glob', { spy: true });
|
||||
|
||||
// Mock the child_process module to control grep/git grep behavior
|
||||
vi.mock('child_process', () => ({
|
||||
|
@ -223,6 +227,15 @@ describe('GrepTool', () => {
|
|||
/params must have required property 'pattern'/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return a GREP_EXECUTION_ERROR on failure', async () => {
|
||||
vi.mocked(glob.globStream).mockRejectedValue(new Error('Glob failed'));
|
||||
const params: GrepToolParams = { pattern: 'hello' };
|
||||
const invocation = grepTool.build(params);
|
||||
const result = await invocation.execute(abortSignal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.GREP_EXECUTION_ERROR);
|
||||
vi.mocked(glob.globStream).mockReset();
|
||||
});
|
||||
});
|
||||
|
||||
describe('multi-directory workspace', () => {
|
||||
|
|
|
@ -21,6 +21,7 @@ import { makeRelative, shortenPath } from '../utils/paths.js';
|
|||
import { getErrorMessage, isNodeError } from '../utils/errors.js';
|
||||
import { isGitRepository } from '../utils/gitUtils.js';
|
||||
import { Config } from '../config/config.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
// --- Interfaces ---
|
||||
|
||||
|
@ -198,6 +199,10 @@ class GrepToolInvocation extends BaseToolInvocation<
|
|||
return {
|
||||
llmContent: `Error during grep search operation: ${errorMessage}`,
|
||||
returnDisplay: `Error: ${errorMessage}`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.GREP_EXECUTION_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
@ -23,6 +23,7 @@ import { LSTool } from './ls.js';
|
|||
import { Config } from '../config/config.js';
|
||||
import { WorkspaceContext } from '../utils/workspaceContext.js';
|
||||
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
describe('LSTool', () => {
|
||||
let lsTool: LSTool;
|
||||
|
@ -288,6 +289,7 @@ describe('LSTool', () => {
|
|||
|
||||
expect(result.llmContent).toContain('Path is not a directory');
|
||||
expect(result.returnDisplay).toBe('Error: Path is not a directory.');
|
||||
expect(result.error?.type).toBe(ToolErrorType.PATH_IS_NOT_A_DIRECTORY);
|
||||
});
|
||||
|
||||
it('should handle non-existent paths', async () => {
|
||||
|
@ -302,6 +304,7 @@ describe('LSTool', () => {
|
|||
|
||||
expect(result.llmContent).toContain('Error listing directory');
|
||||
expect(result.returnDisplay).toBe('Error: Failed to list directory.');
|
||||
expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR);
|
||||
});
|
||||
|
||||
it('should sort directories first, then files alphabetically', async () => {
|
||||
|
@ -357,6 +360,7 @@ describe('LSTool', () => {
|
|||
expect(result.llmContent).toContain('Error listing directory');
|
||||
expect(result.llmContent).toContain('permission denied');
|
||||
expect(result.returnDisplay).toBe('Error: Failed to list directory.');
|
||||
expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR);
|
||||
});
|
||||
|
||||
it('should throw for invalid params at build time', async () => {
|
||||
|
|
|
@ -15,6 +15,7 @@ import {
|
|||
} from './tools.js';
|
||||
import { makeRelative, shortenPath } from '../utils/paths.js';
|
||||
import { Config, DEFAULT_FILE_FILTERING_OPTIONS } from '../config/config.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
/**
|
||||
* Parameters for the LS tool
|
||||
|
@ -114,11 +115,19 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
|
|||
}
|
||||
|
||||
// Helper for consistent error formatting
|
||||
private errorResult(llmContent: string, returnDisplay: string): ToolResult {
|
||||
private errorResult(
|
||||
llmContent: string,
|
||||
returnDisplay: string,
|
||||
type: ToolErrorType,
|
||||
): ToolResult {
|
||||
return {
|
||||
llmContent,
|
||||
// Keep returnDisplay simpler in core logic
|
||||
returnDisplay: `Error: ${returnDisplay}`,
|
||||
error: {
|
||||
message: llmContent,
|
||||
type,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
@ -135,12 +144,14 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
|
|||
return this.errorResult(
|
||||
`Error: Directory not found or inaccessible: ${this.params.path}`,
|
||||
`Directory not found or inaccessible.`,
|
||||
ToolErrorType.FILE_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
if (!stats.isDirectory()) {
|
||||
return this.errorResult(
|
||||
`Error: Path is not a directory: ${this.params.path}`,
|
||||
`Path is not a directory.`,
|
||||
ToolErrorType.PATH_IS_NOT_A_DIRECTORY,
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -253,7 +264,11 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
|
|||
};
|
||||
} catch (error) {
|
||||
const errorMsg = `Error listing directory: ${error instanceof Error ? error.message : String(error)}`;
|
||||
return this.errorResult(errorMsg, 'Failed to list directory.');
|
||||
return this.errorResult(
|
||||
errorMsg,
|
||||
'Failed to list directory.',
|
||||
ToolErrorType.LS_EXECUTION_ERROR,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -17,6 +17,7 @@ import {
|
|||
import { DiscoveredMCPTool, generateValidName } from './mcp-tool.js'; // Added getStringifiedResultForDisplay
|
||||
import { ToolResult, ToolConfirmationOutcome } from './tools.js'; // Added ToolConfirmationOutcome
|
||||
import { CallableTool, Part } from '@google/genai';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
// Mock @google/genai mcpToTool and CallableTool
|
||||
// We only need to mock the parts of CallableTool that DiscoveredMCPTool uses.
|
||||
|
@ -189,7 +190,7 @@ describe('DiscoveredMCPTool', () => {
|
|||
{ isErrorValue: true, description: 'true (bool)' },
|
||||
{ isErrorValue: 'true', description: '"true" (str)' },
|
||||
])(
|
||||
'should consider a ToolResult with isError $description to be a failure',
|
||||
'should return a structured error if MCP tool reports an error',
|
||||
async ({ isErrorValue }) => {
|
||||
const tool = new DiscoveredMCPTool(
|
||||
mockCallableToolInstance,
|
||||
|
@ -210,16 +211,18 @@ describe('DiscoveredMCPTool', () => {
|
|||
},
|
||||
];
|
||||
mockCallTool.mockResolvedValue(mockMcpToolResponseParts);
|
||||
const expectedError = new Error(
|
||||
`MCP tool '${serverToolName}' reported tool error with response: ${JSON.stringify(
|
||||
mockMcpToolResponseParts,
|
||||
)}`,
|
||||
);
|
||||
const expectedErrorMessage = `MCP tool '${serverToolName}' reported tool error with response: ${JSON.stringify(
|
||||
mockMcpToolResponseParts,
|
||||
)}`;
|
||||
|
||||
const invocation = tool.build(params);
|
||||
await expect(
|
||||
invocation.execute(new AbortController().signal),
|
||||
).rejects.toThrow(expectedError);
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
|
||||
expect(result.error?.type).toBe(ToolErrorType.MCP_TOOL_ERROR);
|
||||
expect(result.llmContent).toBe(expectedErrorMessage);
|
||||
expect(result.returnDisplay).toContain(
|
||||
`Error: MCP tool '${serverToolName}' reported an error.`,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
|
|
|
@ -16,6 +16,7 @@ import {
|
|||
ToolResult,
|
||||
} from './tools.js';
|
||||
import { CallableTool, FunctionCall, Part } from '@google/genai';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
type ToolParams = Record<string, unknown>;
|
||||
|
||||
|
@ -139,9 +140,19 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation<
|
|||
|
||||
// Ensure the response is not an error
|
||||
if (this.isMCPToolError(rawResponseParts)) {
|
||||
throw new Error(
|
||||
`MCP tool '${this.serverToolName}' reported tool error with response: ${JSON.stringify(rawResponseParts)}`,
|
||||
);
|
||||
const errorMessage = `MCP tool '${
|
||||
this.serverToolName
|
||||
}' reported tool error with response: ${JSON.stringify(
|
||||
rawResponseParts,
|
||||
)}`;
|
||||
return {
|
||||
llmContent: errorMessage,
|
||||
returnDisplay: `Error: MCP tool '${this.serverToolName}' reported an error.`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.MCP_TOOL_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const transformedParts = transformMcpContentToParts(rawResponseParts);
|
||||
|
|
|
@ -16,6 +16,7 @@ import * as fs from 'fs/promises';
|
|||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import { ToolConfirmationOutcome } from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock(import('fs/promises'), async (importOriginal) => {
|
||||
|
@ -287,6 +288,9 @@ describe('MemoryTool', () => {
|
|||
expect(result.returnDisplay).toBe(
|
||||
`Error saving memory: ${underlyingError.message}`,
|
||||
);
|
||||
expect(result.error?.type).toBe(
|
||||
ToolErrorType.MEMORY_TOOL_EXECUTION_ERROR,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -20,6 +20,7 @@ import * as Diff from 'diff';
|
|||
import { DEFAULT_DIFF_OPTIONS } from './diffOptions.js';
|
||||
import { tildeifyPath } from '../utils/paths.js';
|
||||
import { ModifiableDeclarativeTool, ModifyContext } from './modifiable-tool.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
const memoryToolSchemaData: FunctionDeclaration = {
|
||||
name: 'save_memory',
|
||||
|
@ -273,6 +274,10 @@ class MemoryToolInvocation extends BaseToolInvocation<
|
|||
error: `Failed to save memory. Detail: ${errorMessage}`,
|
||||
}),
|
||||
returnDisplay: `Error saving memory: ${errorMessage}`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.MEMORY_TOOL_EXECUTION_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
@ -15,6 +15,10 @@ import os from 'os';
|
|||
import { Config } from '../config/config.js';
|
||||
import { WorkspaceContext } from '../utils/workspaceContext.js';
|
||||
import { StandardFileSystemService } from '../services/fileSystemService.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import * as glob from 'glob';
|
||||
|
||||
vi.mock('glob', { spy: true });
|
||||
|
||||
vi.mock('mime-types', () => {
|
||||
const lookup = (filename: string) => {
|
||||
|
@ -566,6 +570,28 @@ Content of file[1]
|
|||
});
|
||||
});
|
||||
|
||||
describe('Error handling', () => {
|
||||
it('should return an INVALID_TOOL_PARAMS error if no paths are provided', async () => {
|
||||
const params = { paths: [], include: [] };
|
||||
expect(() => {
|
||||
tool.build(params);
|
||||
}).toThrow('params/paths must NOT have fewer than 1 items');
|
||||
});
|
||||
|
||||
it('should return a READ_MANY_FILES_SEARCH_ERROR on glob failure', async () => {
|
||||
vi.mocked(glob.glob).mockRejectedValue(new Error('Glob failed'));
|
||||
const params = { paths: ['*.txt'] };
|
||||
const invocation = tool.build(params);
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
expect(result.error?.type).toBe(
|
||||
ToolErrorType.READ_MANY_FILES_SEARCH_ERROR,
|
||||
);
|
||||
expect(result.llmContent).toBe('Error during file search: Glob failed');
|
||||
// Reset glob.
|
||||
vi.mocked(glob.glob).mockReset();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Batch Processing', () => {
|
||||
const createMultipleFiles = (count: number, contentPrefix = 'Content') => {
|
||||
const files: string[] = [];
|
||||
|
|
|
@ -29,6 +29,7 @@ import {
|
|||
recordFileOperationMetric,
|
||||
FileOperation,
|
||||
} from '../telemetry/metrics.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
/**
|
||||
* Parameters for the ReadManyFilesTool.
|
||||
|
@ -232,13 +233,6 @@ ${finalExclusionPatternsForDescription
|
|||
: [...exclude];
|
||||
|
||||
const searchPatterns = [...inputPatterns, ...include];
|
||||
if (searchPatterns.length === 0) {
|
||||
return {
|
||||
llmContent: 'No search paths or include patterns provided.',
|
||||
returnDisplay: `## Information\n\nNo search paths or include patterns were specified. Nothing to read or concatenate.`,
|
||||
};
|
||||
}
|
||||
|
||||
try {
|
||||
const allEntries = new Set<string>();
|
||||
const workspaceDirs = this.config.getWorkspaceContext().getDirectories();
|
||||
|
@ -352,9 +346,14 @@ ${finalExclusionPatternsForDescription
|
|||
});
|
||||
}
|
||||
} catch (error) {
|
||||
const errorMessage = `Error during file search: ${getErrorMessage(error)}`;
|
||||
return {
|
||||
llmContent: `Error during file search: ${getErrorMessage(error)}`,
|
||||
llmContent: errorMessage,
|
||||
returnDisplay: `## File Search Error\n\nAn error occurred while searching for files:\n\`\`\`\n${getErrorMessage(error)}\n\`\`\``,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.READ_MANY_FILES_SEARCH_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
@ -24,10 +24,43 @@ export enum ToolErrorType {
|
|||
PERMISSION_DENIED = 'permission_denied',
|
||||
NO_SPACE_LEFT = 'no_space_left',
|
||||
TARGET_IS_DIRECTORY = 'target_is_directory',
|
||||
PATH_NOT_IN_WORKSPACE = 'path_not_in_workspace',
|
||||
SEARCH_PATH_NOT_FOUND = 'search_path_not_found',
|
||||
SEARCH_PATH_NOT_A_DIRECTORY = 'search_path_not_a_directory',
|
||||
|
||||
// Edit-specific Errors
|
||||
EDIT_PREPARATION_FAILURE = 'edit_preparation_failure',
|
||||
EDIT_NO_OCCURRENCE_FOUND = 'edit_no_occurrence_found',
|
||||
EDIT_EXPECTED_OCCURRENCE_MISMATCH = 'edit_expected_occurrence_mismatch',
|
||||
EDIT_NO_CHANGE = 'edit_no_change',
|
||||
|
||||
// Glob-specific Errors
|
||||
GLOB_EXECUTION_ERROR = 'glob_execution_error',
|
||||
|
||||
// Grep-specific Errors
|
||||
GREP_EXECUTION_ERROR = 'grep_execution_error',
|
||||
|
||||
// Ls-specific Errors
|
||||
LS_EXECUTION_ERROR = 'ls_execution_error',
|
||||
PATH_IS_NOT_A_DIRECTORY = 'path_is_not_a_directory',
|
||||
|
||||
// MCP-specific Errors
|
||||
MCP_TOOL_ERROR = 'mcp_tool_error',
|
||||
|
||||
// Memory-specific Errors
|
||||
MEMORY_TOOL_EXECUTION_ERROR = 'memory_tool_execution_error',
|
||||
|
||||
// ReadManyFiles-specific Errors
|
||||
READ_MANY_FILES_SEARCH_ERROR = 'read_many_files_search_error',
|
||||
|
||||
// DiscoveredTool-specific Errors
|
||||
DISCOVERED_TOOL_EXECUTION_ERROR = 'discovered_tool_execution_error',
|
||||
|
||||
// WebFetch-specific Errors
|
||||
WEB_FETCH_NO_URL_IN_PROMPT = 'web_fetch_no_url_in_prompt',
|
||||
WEB_FETCH_FALLBACK_FAILED = 'web_fetch_fallback_failed',
|
||||
WEB_FETCH_PROCESSING_ERROR = 'web_fetch_processing_error',
|
||||
|
||||
// WebSearch-specific Errors
|
||||
WEB_SEARCH_FAILED = 'web_search_failed',
|
||||
}
|
||||
|
|
|
@ -24,6 +24,7 @@ import fs from 'node:fs';
|
|||
import { MockTool } from '../test-utils/tools.js';
|
||||
|
||||
import { McpClientManager } from './mcp-client-manager.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
vi.mock('node:fs');
|
||||
|
||||
|
@ -311,6 +312,81 @@ describe('ToolRegistry', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should return a DISCOVERED_TOOL_EXECUTION_ERROR on tool failure', async () => {
|
||||
const discoveryCommand = 'my-discovery-command';
|
||||
mockConfigGetToolDiscoveryCommand.mockReturnValue(discoveryCommand);
|
||||
vi.spyOn(config, 'getToolCallCommand').mockReturnValue('my-call-command');
|
||||
|
||||
const toolDeclaration: FunctionDeclaration = {
|
||||
name: 'failing-tool',
|
||||
description: 'A tool that fails',
|
||||
parametersJsonSchema: {
|
||||
type: 'object',
|
||||
properties: {},
|
||||
},
|
||||
};
|
||||
|
||||
const mockSpawn = vi.mocked(spawn);
|
||||
// --- Discovery Mock ---
|
||||
const discoveryProcess = {
|
||||
stdout: { on: vi.fn(), removeListener: vi.fn() },
|
||||
stderr: { on: vi.fn(), removeListener: vi.fn() },
|
||||
on: vi.fn(),
|
||||
};
|
||||
mockSpawn.mockReturnValueOnce(discoveryProcess as any);
|
||||
|
||||
discoveryProcess.stdout.on.mockImplementation((event, callback) => {
|
||||
if (event === 'data') {
|
||||
callback(
|
||||
Buffer.from(
|
||||
JSON.stringify([{ functionDeclarations: [toolDeclaration] }]),
|
||||
),
|
||||
);
|
||||
}
|
||||
});
|
||||
discoveryProcess.on.mockImplementation((event, callback) => {
|
||||
if (event === 'close') {
|
||||
callback(0);
|
||||
}
|
||||
});
|
||||
|
||||
await toolRegistry.discoverAllTools();
|
||||
const discoveredTool = toolRegistry.getTool('failing-tool');
|
||||
expect(discoveredTool).toBeDefined();
|
||||
|
||||
// --- Execution Mock ---
|
||||
const executionProcess = {
|
||||
stdout: { on: vi.fn(), removeListener: vi.fn() },
|
||||
stderr: { on: vi.fn(), removeListener: vi.fn() },
|
||||
stdin: { write: vi.fn(), end: vi.fn() },
|
||||
on: vi.fn(),
|
||||
connected: true,
|
||||
disconnect: vi.fn(),
|
||||
removeListener: vi.fn(),
|
||||
};
|
||||
mockSpawn.mockReturnValueOnce(executionProcess as any);
|
||||
|
||||
executionProcess.stderr.on.mockImplementation((event, callback) => {
|
||||
if (event === 'data') {
|
||||
callback(Buffer.from('Something went wrong'));
|
||||
}
|
||||
});
|
||||
executionProcess.on.mockImplementation((event, callback) => {
|
||||
if (event === 'close') {
|
||||
callback(1); // Non-zero exit code
|
||||
}
|
||||
});
|
||||
|
||||
const invocation = (discoveredTool as DiscoveredTool).build({});
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
|
||||
expect(result.error?.type).toBe(
|
||||
ToolErrorType.DISCOVERED_TOOL_EXECUTION_ERROR,
|
||||
);
|
||||
expect(result.llmContent).toContain('Stderr: Something went wrong');
|
||||
expect(result.llmContent).toContain('Exit Code: 1');
|
||||
});
|
||||
|
||||
it('should discover tools using MCP servers defined in getMcpServers', async () => {
|
||||
const discoverSpy = vi.spyOn(
|
||||
McpClientManager.prototype,
|
||||
|
|
|
@ -20,6 +20,7 @@ import { connectAndDiscover } from './mcp-client.js';
|
|||
import { McpClientManager } from './mcp-client-manager.js';
|
||||
import { DiscoveredMCPTool } from './mcp-tool.js';
|
||||
import { parse } from 'shell-quote';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import { safeJsonStringify } from '../utils/safeJsonStringify.js';
|
||||
|
||||
type ToolParams = Record<string, unknown>;
|
||||
|
@ -106,6 +107,10 @@ class DiscoveredToolInvocation extends BaseToolInvocation<
|
|||
return {
|
||||
llmContent,
|
||||
returnDisplay: llmContent,
|
||||
error: {
|
||||
message: llmContent,
|
||||
type: ToolErrorType.DISCOVERED_TOOL_EXECUTION_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
@ -4,17 +4,72 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { WebFetchTool } from './web-fetch.js';
|
||||
import { Config, ApprovalMode } from '../config/config.js';
|
||||
import { ToolConfirmationOutcome } from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import * as fetchUtils from '../utils/fetch.js';
|
||||
|
||||
const mockGenerateContent = vi.fn();
|
||||
const mockGetGeminiClient = vi.fn(() => ({
|
||||
generateContent: mockGenerateContent,
|
||||
}));
|
||||
|
||||
vi.mock('../utils/fetch.js', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof fetchUtils>();
|
||||
return {
|
||||
...actual,
|
||||
fetchWithTimeout: vi.fn(),
|
||||
isPrivateIp: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
describe('WebFetchTool', () => {
|
||||
const mockConfig = {
|
||||
getApprovalMode: vi.fn(),
|
||||
setApprovalMode: vi.fn(),
|
||||
getProxy: vi.fn(),
|
||||
} as unknown as Config;
|
||||
let mockConfig: Config;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
mockConfig = {
|
||||
getApprovalMode: vi.fn(),
|
||||
setApprovalMode: vi.fn(),
|
||||
getProxy: vi.fn(),
|
||||
getGeminiClient: mockGetGeminiClient,
|
||||
} as unknown as Config;
|
||||
});
|
||||
|
||||
describe('execute', () => {
|
||||
it('should return WEB_FETCH_NO_URL_IN_PROMPT when no URL is in the prompt for fallback', async () => {
|
||||
vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(true);
|
||||
const tool = new WebFetchTool(mockConfig);
|
||||
const params = { prompt: 'no url here' };
|
||||
expect(() => tool.build(params)).toThrow(
|
||||
"The 'prompt' must contain at least one valid URL (starting with http:// or https://).",
|
||||
);
|
||||
});
|
||||
|
||||
it('should return WEB_FETCH_FALLBACK_FAILED on fallback fetch failure', async () => {
|
||||
vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(true);
|
||||
vi.spyOn(fetchUtils, 'fetchWithTimeout').mockRejectedValue(
|
||||
new Error('fetch failed'),
|
||||
);
|
||||
const tool = new WebFetchTool(mockConfig);
|
||||
const params = { prompt: 'fetch https://private.ip' };
|
||||
const invocation = tool.build(params);
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.WEB_FETCH_FALLBACK_FAILED);
|
||||
});
|
||||
|
||||
it('should return WEB_FETCH_PROCESSING_ERROR on general processing failure', async () => {
|
||||
vi.spyOn(fetchUtils, 'isPrivateIp').mockReturnValue(false);
|
||||
mockGenerateContent.mockRejectedValue(new Error('API error'));
|
||||
const tool = new WebFetchTool(mockConfig);
|
||||
const params = { prompt: 'fetch https://public.ip' };
|
||||
const invocation = tool.build(params);
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
expect(result.error?.type).toBe(ToolErrorType.WEB_FETCH_PROCESSING_ERROR);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldConfirmExecute', () => {
|
||||
it('should return confirmation details with the correct prompt and urls', async () => {
|
||||
|
@ -58,10 +113,10 @@ describe('WebFetchTool', () => {
|
|||
});
|
||||
|
||||
it('should return false if approval mode is AUTO_EDIT', async () => {
|
||||
const tool = new WebFetchTool({
|
||||
...mockConfig,
|
||||
getApprovalMode: () => ApprovalMode.AUTO_EDIT,
|
||||
} as unknown as Config);
|
||||
vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue(
|
||||
ApprovalMode.AUTO_EDIT,
|
||||
);
|
||||
const tool = new WebFetchTool(mockConfig);
|
||||
const params = { prompt: 'fetch https://example.com' };
|
||||
const invocation = tool.build(params);
|
||||
const confirmationDetails = await invocation.shouldConfirmExecute(
|
||||
|
@ -72,11 +127,7 @@ describe('WebFetchTool', () => {
|
|||
});
|
||||
|
||||
it('should call setApprovalMode when onConfirm is called with ProceedAlways', async () => {
|
||||
const setApprovalMode = vi.fn();
|
||||
const tool = new WebFetchTool({
|
||||
...mockConfig,
|
||||
setApprovalMode,
|
||||
} as unknown as Config);
|
||||
const tool = new WebFetchTool(mockConfig);
|
||||
const params = { prompt: 'fetch https://example.com' };
|
||||
const invocation = tool.build(params);
|
||||
const confirmationDetails = await invocation.shouldConfirmExecute(
|
||||
|
@ -93,7 +144,9 @@ describe('WebFetchTool', () => {
|
|||
);
|
||||
}
|
||||
|
||||
expect(setApprovalMode).toHaveBeenCalledWith(ApprovalMode.AUTO_EDIT);
|
||||
expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
|
||||
ApprovalMode.AUTO_EDIT,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -13,6 +13,7 @@ import {
|
|||
ToolInvocation,
|
||||
ToolResult,
|
||||
} from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import { getErrorMessage } from '../utils/errors.js';
|
||||
import { ApprovalMode, Config } from '../config/config.js';
|
||||
import { getResponseText } from '../utils/generateContentResponseUtilities.js';
|
||||
|
@ -73,12 +74,6 @@ class WebFetchToolInvocation extends BaseToolInvocation<
|
|||
|
||||
private async executeFallback(signal: AbortSignal): Promise<ToolResult> {
|
||||
const urls = extractUrls(this.params.prompt);
|
||||
if (urls.length === 0) {
|
||||
return {
|
||||
llmContent: 'Error: No URL found in the prompt for fallback.',
|
||||
returnDisplay: 'Error: No URL found in the prompt for fallback.',
|
||||
};
|
||||
}
|
||||
// For now, we only support one URL for fallback
|
||||
let url = urls[0];
|
||||
|
||||
|
@ -130,6 +125,10 @@ ${textContent}
|
|||
return {
|
||||
llmContent: `Error: ${errorMessage}`,
|
||||
returnDisplay: `Error: ${errorMessage}`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.WEB_FETCH_FALLBACK_FAILED,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
@ -300,6 +299,10 @@ ${sourceListFormatted.join('\n')}`;
|
|||
return {
|
||||
llmContent: `Error: ${errorMessage}`,
|
||||
returnDisplay: `Error: ${errorMessage}`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.WEB_FETCH_PROCESSING_ERROR,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest';
|
|||
import { WebSearchTool, WebSearchToolParams } from './web-search.js';
|
||||
import { Config } from '../config/config.js';
|
||||
import { GeminiClient } from '../core/client.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
// Mock GeminiClient and Config constructor
|
||||
vi.mock('../core/client.js');
|
||||
|
@ -112,7 +113,7 @@ describe('WebSearchTool', () => {
|
|||
expect(result.returnDisplay).toBe('No information found.');
|
||||
});
|
||||
|
||||
it('should handle API errors gracefully', async () => {
|
||||
it('should return a WEB_SEARCH_FAILED error on failure', async () => {
|
||||
const params: WebSearchToolParams = { query: 'error query' };
|
||||
const testError = new Error('API Failure');
|
||||
(mockGeminiClient.generateContent as Mock).mockRejectedValue(testError);
|
||||
|
@ -120,6 +121,7 @@ describe('WebSearchTool', () => {
|
|||
const invocation = tool.build(params);
|
||||
const result = await invocation.execute(abortSignal);
|
||||
|
||||
expect(result.error?.type).toBe(ToolErrorType.WEB_SEARCH_FAILED);
|
||||
expect(result.llmContent).toContain('Error:');
|
||||
expect(result.llmContent).toContain('API Failure');
|
||||
expect(result.returnDisplay).toBe('Error performing web search.');
|
||||
|
|
|
@ -12,6 +12,7 @@ import {
|
|||
ToolInvocation,
|
||||
ToolResult,
|
||||
} from './tools.js';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
|
||||
import { getErrorMessage } from '../utils/errors.js';
|
||||
import { Config } from '../config/config.js';
|
||||
|
@ -153,6 +154,10 @@ class WebSearchToolInvocation extends BaseToolInvocation<
|
|||
return {
|
||||
llmContent: `Error: ${errorMessage}`,
|
||||
returnDisplay: `Error performing web search.`,
|
||||
error: {
|
||||
message: errorMessage,
|
||||
type: ToolErrorType.WEB_SEARCH_FAILED,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue