diff --git a/core/config/load.ts b/core/config/load.ts index f7bb018349d..30df0fd7503 100644 --- a/core/config/load.ts +++ b/core/config/load.ts @@ -58,7 +58,7 @@ import { import { localPathToUri } from "../util/pathToUri"; import CustomContextProviderClass from "../context/providers/CustomContextProvider"; -import { getToolsForIde } from "../tools"; +import { getBaseToolDefinitions } from "../tools"; import { resolveRelativePathInDir } from "../util/ideUtils"; import { getWorkspaceRcConfigs } from "./json/loadRcConfigs"; import { loadConfigContextProviders } from "./loadContextProviders"; @@ -498,7 +498,7 @@ async function intermediateToFinalConfig({ const continueConfig: ContinueConfig = { ...config, contextProviders, - tools: await getToolsForIde(ide), + tools: getBaseToolDefinitions(), mcpServerStatuses: [], slashCommands: [], modelsByRole: { diff --git a/core/config/profile/doLoadConfig.ts b/core/config/profile/doLoadConfig.ts index 4b0f6450091..0b39489ebb2 100644 --- a/core/config/profile/doLoadConfig.ts +++ b/core/config/profile/doLoadConfig.ts @@ -282,6 +282,8 @@ export default async function doLoadConfig(options: { enableExperimentalTools: newConfig.experimental?.enableExperimentalTools ?? false, isSignedIn, + isRemote: await ide.isWorkspaceRemote(), + modelName: newConfig.selectedModelByRole.chat?.model, }), ); diff --git a/core/config/yaml/loadYaml.ts b/core/config/yaml/loadYaml.ts index 694c9e4da85..7a9c9f63404 100644 --- a/core/config/yaml/loadYaml.ts +++ b/core/config/yaml/loadYaml.ts @@ -25,7 +25,7 @@ import { modifyAnyConfigWithSharedConfig } from "../sharedConfig"; import { convertPromptBlockToSlashCommand } from "../../commands/slash/promptBlockSlashCommand"; import { slashCommandFromPromptFile } from "../../commands/slash/promptFileSlashCommand"; import { getControlPlaneEnvSync } from "../../control-plane/env"; -import { getToolsForIde } from "../../tools"; +import { getBaseToolDefinitions } from "../../tools"; import { getCleanUriPath } from "../../util/uri"; import { loadConfigContextProviders } from "../loadContextProviders"; import { getAllDotContinueDefinitionFiles } from "../loadLocalAssistants"; @@ -168,7 +168,7 @@ async function configYamlToContinueConfig(options: { const continueConfig: ContinueConfig = { slashCommands: [], - tools: await getToolsForIde(ide), + tools: getBaseToolDefinitions(), mcpServerStatuses: [], contextProviders: [], modelsByRole: { diff --git a/core/index.d.ts b/core/index.d.ts index 90039f02537..d639583efe7 100644 --- a/core/index.d.ts +++ b/core/index.d.ts @@ -1112,6 +1112,8 @@ export interface ConfigDependentToolParams { rules: RuleWithSource[]; enableExperimentalTools: boolean; isSignedIn: boolean; + isRemote: boolean; + modelName: string | undefined; } export type GetTool = (params: ConfigDependentToolParams) => Tool; diff --git a/core/llm/toolSupport.ts b/core/llm/toolSupport.ts index ad8ece0d6bd..ed40733a579 100644 --- a/core/llm/toolSupport.ts +++ b/core/llm/toolSupport.ts @@ -375,6 +375,7 @@ export function isRecommendedAgentModel(modelName: string): boolean { [/deepseek/, /r1|reasoner/], [/gemini/, /2\.5/, /pro/], [/gpt/, /4/], + [/gpt-5/], [/claude/, /sonnet/, /3\.5|3\.7|3-5|3-7|-4/], [/claude/, /opus/, /-4/], ]; diff --git a/core/tools/builtIn.ts b/core/tools/builtIn.ts index c5c5da16c7c..6e8ed106234 100644 --- a/core/tools/builtIn.ts +++ b/core/tools/builtIn.ts @@ -2,6 +2,8 @@ export enum BuiltInToolNames { ReadFile = "read_file", EditExistingFile = "edit_existing_file", SearchAndReplaceInFile = "search_and_replace_in_file", + SingleFindAndReplace = "single_find_and_replace", + MultiEdit = "multi_edit", ReadCurrentlyOpenFile = "read_currently_open_file", CreateNewFile = "create_new_file", RunTerminalCommand = "run_terminal_command", @@ -25,4 +27,6 @@ export const BUILT_IN_GROUP_NAME = "Built-In"; export const CLIENT_TOOLS_IMPLS = [ BuiltInToolNames.EditExistingFile, BuiltInToolNames.SearchAndReplaceInFile, + BuiltInToolNames.SingleFindAndReplace, + BuiltInToolNames.MultiEdit, ]; diff --git a/core/tools/definitions/index.ts b/core/tools/definitions/index.ts index c1998444c92..6313eb9fc46 100644 --- a/core/tools/definitions/index.ts +++ b/core/tools/definitions/index.ts @@ -6,12 +6,14 @@ export { fetchUrlContentTool } from "./fetchUrlContent"; export { globSearchTool } from "./globSearch"; export { grepSearchTool } from "./grepSearch"; export { lsTool } from "./ls"; +export { multiEditTool } from "./multiEdit"; export { readCurrentlyOpenFileTool } from "./readCurrentlyOpenFile"; export { readFileTool } from "./readFile"; export { requestRuleTool } from "./requestRule"; export { runTerminalCommandTool } from "./runTerminalCommand"; export { searchAndReplaceInFileTool } from "./searchAndReplaceInFile"; export { searchWebTool } from "./searchWeb"; +export { singleFindAndReplaceTool } from "./singleFindAndReplace"; export { viewDiffTool } from "./viewDiff"; export { viewRepoMapTool } from "./viewRepoMap"; export { viewSubdirectoryTool } from "./viewSubdirectory"; diff --git a/core/tools/definitions/multiEdit.ts b/core/tools/definitions/multiEdit.ts new file mode 100644 index 00000000000..3003120000f --- /dev/null +++ b/core/tools/definitions/multiEdit.ts @@ -0,0 +1,131 @@ +import { Tool } from "../.."; +import { BUILT_IN_GROUP_NAME, BuiltInToolNames } from "../builtIn"; + +export interface EditOperation { + old_string: string; + new_string: string; + replace_all?: boolean; +} + +export interface MultiEditArgs { + filepath: string; + edits: EditOperation[]; +} + +export const multiEditTool: Tool = { + type: "function", + displayTitle: "Multi Edit", + wouldLikeTo: "edit {{{ filepath }}}", + isCurrently: "editing {{{ filepath }}}", + hasAlready: "edited {{{ filepath }}}", + group: BUILT_IN_GROUP_NAME, + readonly: false, + isInstant: false, + function: { + name: BuiltInToolNames.MultiEdit, + description: `This is a tool for making multiple edits to a single file in one operation. It +is built on top of the single find and replace tool and allows you to perform multiple +find-and-replace operations efficiently. Prefer this tool over the single find and replace tool +when you need to make multiple edits to the same file. + +Before using this tool: + +1. Use the read_file tool to understand the file's contents and context +2. Verify the directory path is correct + +To make multiple file edits, provide the following: + +1. filepath: The path to the file to modify (relative to the root of the workspace) +2. edits: An array of edit operations to perform, where each edit contains: + - old_string: The text to replace (must match the file contents exactly, including all whitespace and indentation) + - new_string: The edited text to replace the old_string + - replace_all: Replace all occurrences of old_string. This parameter is optional and defaults to false. + +IMPORTANT: +- All edits are applied in sequence, in the order they are provided +- Each edit operates on the result of the previous edit +- All edits must be valid for the operation to succeed - if any edit fails, +none will be applied +- This tool is ideal when you need to make several changes to different parts +of the same file + +CRITICAL REQUIREMENTS: + +1. All edits follow the same requirements as the single find and replace tool +2. The edits are atomic - either all succeed or none are applied +3. Plan your edits carefully to avoid conflicts between sequential operations + +WARNING: + +- The tool will fail if edits.old_string doesn't match the file contents +exactly (including whitespace) +- The tool will fail if edits.old_string and edits.new_string are the same +- Since edits are applied in sequence, ensure that earlier edits don't affect +the text that later edits are trying to find + +When making edits: + +- Ensure all edits result in idiomatic, correct code +- Do not leave the code in a broken state +- Only use emojis if the user explicitly requests it. Avoid adding emojis to +files unless asked. +- Use replace_all for replacing and renaming strings across the file. This +parameter is useful if you want to rename a variable for instance. + +If you want to create a new file, use: +- A new file path +- First edit: empty old_string and the new file's contents as new_string +- Subsequent edits are not allowed - there is no need since you are creating`, + parameters: { + type: "object", + required: ["filepath", "edits"], + properties: { + filepath: { + type: "string", + description: + "The path to the file to modify, relative to the root of the workspace", + }, + edits: { + type: "array", + description: + "Array of edit operations to perform sequentially on the file", + items: { + type: "object", + required: ["old_string", "new_string"], + properties: { + old_string: { + type: "string", + description: "The text to replace", + }, + new_string: { + type: "string", + description: "The text to replace it with", + }, + replace_all: { + type: "boolean", + description: + "Replace all occurrences of old_string (default false)", + }, + }, + }, + }, + }, + }, + }, + systemMessageDescription: { + prefix: `To make multiple edits to a single file, use the ${BuiltInToolNames.MultiEdit} tool with a filepath (relative to the root of the workspace) and an array of edit operations. + + For example, you could respond with:`, + exampleArgs: [ + ["filepath", "path/to/file.ts"], + [ + "edits", + `[ + { "old_string": "const oldVar = 'value'", "new_string": "const newVar = 'updated'" }, + { "old_string": "oldFunction()", "new_string": "newFunction()", "replace_all": true } +]`, + ], + ], + }, + defaultToolPolicy: "allowedWithPermission", +}; diff --git a/core/tools/definitions/singleFindAndReplace.ts b/core/tools/definitions/singleFindAndReplace.ts new file mode 100644 index 00000000000..b5537d77523 --- /dev/null +++ b/core/tools/definitions/singleFindAndReplace.ts @@ -0,0 +1,84 @@ +import { Tool } from "../.."; +import { BUILT_IN_GROUP_NAME, BuiltInToolNames } from "../builtIn"; + +export interface SingleFindAndReplaceArgs { + filepath: string; + old_string: string; + new_string: string; + replace_all?: boolean; +} + +export const singleFindAndReplaceTool: Tool = { + type: "function", + displayTitle: "Find and Replace", + wouldLikeTo: "find and replace in {{{ filepath }}}", + isCurrently: "finding and replacing in {{{ filepath }}}", + hasAlready: "found and replaced in {{{ filepath }}}", + group: BUILT_IN_GROUP_NAME, + readonly: false, + isInstant: false, + function: { + name: BuiltInToolNames.SingleFindAndReplace, + description: `Performs exact string replacements in files. + +Usage: + +- You must use your \`read_file\` tool at least once in the conversation before +editing. This tool will error if you attempt an edit without reading the file. + +- When editing text from read_file tool output, ensure you preserve the exact +indentation (tabs/spaces) as it appears AFTER the line number prefix. The line +number prefix format is: spaces + line number + tab. Everything after that tab +is the actual file content to match. Never include any part of the line number +prefix in the old_string or new_string. + +- ALWAYS prefer editing existing files in the codebase. NEVER write new files +unless explicitly required. + +- Only use emojis if the user explicitly requests it. Avoid adding emojis to +files unless asked. + +- The edit will FAIL if \`old_string\` is not unique in the file. Either provide +a larger string with more surrounding context to make it unique or use +\`replace_all\` to change every instance of \`old_string\`. + +- Use \`replace_all\` for replacing and renaming strings across the file. This +parameter is useful if you want to rename a variable for instance.`, + parameters: { + type: "object", + required: ["filepath", "old_string", "new_string"], + properties: { + filepath: { + type: "string", + description: + "The path to the file to modify, relative to the root of the workspace", + }, + old_string: { + type: "string", + description: "The text to replace", + }, + new_string: { + type: "string", + description: + "The text to replace it with (must be different from old_string)", + }, + replace_all: { + type: "boolean", + description: "Replace all occurrences of old_string (default false)", + }, + }, + }, + }, + systemMessageDescription: { + prefix: `To perform exact string replacements in files, use the ${BuiltInToolNames.SingleFindAndReplace} tool with a filepath (relative to the root of the workspace) and the strings to find and replace. + + For example, you could respond with:`, + exampleArgs: [ + ["filepath", "path/to/file.ts"], + ["old_string", "const oldVariable = 'value'"], + ["new_string", "const newVariable = 'updated'"], + ["replace_all", "false"], + ], + }, + defaultToolPolicy: "allowedWithPermission", +}; diff --git a/core/tools/definitions/toolDefinitions.test.ts b/core/tools/definitions/toolDefinitions.test.ts index abeac08ef31..265ebe5e12f 100644 --- a/core/tools/definitions/toolDefinitions.test.ts +++ b/core/tools/definitions/toolDefinitions.test.ts @@ -8,6 +8,8 @@ describe("Tool Definitions", () => { rules: [], enableExperimentalTools: false, isSignedIn: false, + isRemote: false, + modelName: "a model", }; // Helper function to get the actual tool object diff --git a/core/tools/index.ts b/core/tools/index.ts index 850d89349cd..fa66c4c915e 100644 --- a/core/tools/index.ts +++ b/core/tools/index.ts @@ -1,12 +1,8 @@ -import { ConfigDependentToolParams, IDE, Tool } from ".."; +import { ConfigDependentToolParams, Tool } from ".."; import * as toolDefinitions from "./definitions"; // I'm writing these as functions because we've messed up 3 TIMES by pushing to const, causing duplicate tool definitions on subsequent config loads. - -// missing support for remote os calls: https://github.com/microsoft/vscode/issues/252269 -const getLocalOnlyToolDefinitions = () => [toolDefinitions.grepSearchTool]; - -const getBaseToolDefinitions = () => [ +export const getBaseToolDefinitions = () => [ toolDefinitions.readFileTool, toolDefinitions.createNewFileTool, toolDefinitions.runTerminalCommandTool, @@ -16,28 +12,42 @@ const getBaseToolDefinitions = () => [ toolDefinitions.lsTool, toolDefinitions.createRuleBlock, toolDefinitions.fetchUrlContentTool, + toolDefinitions.singleFindAndReplaceTool, ]; export const getConfigDependentToolDefinitions = ( params: ConfigDependentToolParams, -): Tool[] => [ - toolDefinitions.requestRuleTool(params), - // Search and replace is now generally available - toolDefinitions.searchAndReplaceInFileTool, - // Keep edit file tool available for models that need it - toolDefinitions.editFileTool, - // Web search is only available for signed-in users - ...(params.isSignedIn ? [toolDefinitions.searchWebTool] : []), - ...(params.enableExperimentalTools - ? [ - toolDefinitions.viewRepoMapTool, - toolDefinitions.viewSubdirectoryTool, - toolDefinitions.codebaseTool, - ] - : []), -]; +): Tool[] => { + const { modelName, isSignedIn, enableExperimentalTools, isRemote } = params; + const tools: Tool[] = []; + + tools.push(toolDefinitions.requestRuleTool(params)); + + if (isSignedIn) { + // Web search is only available for signed-in users + tools.push(toolDefinitions.searchWebTool); + } + + if (enableExperimentalTools) { + tools.push( + toolDefinitions.viewRepoMapTool, + toolDefinitions.viewSubdirectoryTool, + toolDefinitions.codebaseTool, + ); + } + + // OLD SEARCH AND REPLACE IS CURRENTLY NOT USED + // toolDefinitions.searchAndReplaceInFileTool + if (modelName?.includes("claude") || modelName?.includes("gpt-5")) { + tools.push(toolDefinitions.multiEditTool); + } else { + tools.push(toolDefinitions.editFileTool); + } + + // missing support for remote os calls: https://github.com/microsoft/vscode/issues/252269 + if (!isRemote) { + tools.push(toolDefinitions.grepSearchTool); + } -export const getToolsForIde = async (ide: IDE): Promise => - (await ide.isWorkspaceRemote()) - ? getBaseToolDefinitions() - : [...getBaseToolDefinitions(), ...getLocalOnlyToolDefinitions()]; + return tools; +}; diff --git a/core/tools/searchWebGating.vitest.ts b/core/tools/searchWebGating.vitest.ts index e68acdd52a3..4ebb6e5b45f 100644 --- a/core/tools/searchWebGating.vitest.ts +++ b/core/tools/searchWebGating.vitest.ts @@ -1,6 +1,6 @@ -import { test, expect } from "vitest"; -import { getConfigDependentToolDefinitions } from "./index"; +import { expect, test } from "vitest"; import { BuiltInToolNames } from "./builtIn"; +import { getConfigDependentToolDefinitions } from "./index"; test("searchWeb tool is only available when user is signed in", () => { // Test with signed-in user @@ -8,6 +8,8 @@ test("searchWeb tool is only available when user is signed in", () => { rules: [], enableExperimentalTools: false, isSignedIn: true, + isRemote: false, + modelName: "", }); const searchWebToolSignedIn = signedInTools.find( @@ -21,6 +23,8 @@ test("searchWeb tool is only available when user is signed in", () => { rules: [], enableExperimentalTools: false, isSignedIn: false, + isRemote: false, + modelName: "", }); const searchWebToolNotSignedIn = notSignedInTools.find( diff --git a/gui/package-lock.json b/gui/package-lock.json index a3dffda4cd5..8e4bfb7d9d6 100644 --- a/gui/package-lock.json +++ b/gui/package-lock.json @@ -27,10 +27,12 @@ "@tiptap/react": "^2.1.13", "@tiptap/starter-kit": "^2.1.13", "@tiptap/suggestion": "^2.1.13", + "@types/diff": "^7.0.2", "@types/uuid": "^10.0.0", "anser": "^2.3.2", "clsx": "^2.1.1", "core": "file:../core", + "diff": "^8.0.2", "dompurify": "^3.0.6", "downshift": "^7.6.0", "escape-carriage": "^1.3.1", @@ -129,7 +131,6 @@ "@sentry/esbuild-plugin": "^4.0.2", "@sentry/node": "^9.43.0", "@sentry/vite-plugin": "^4.0.2", - "@typescript-eslint/eslint-plugin": "^7.8.0", "@xenova/transformers": "2.14.0", "adf-to-md": "^1.1.0", "async-mutex": "^0.5.0", @@ -209,6 +210,8 @@ "@types/tar": "^6.1.13", "@types/uuid": "^9.0.7", "@types/win-ca": "^3.5.4", + "@typescript-eslint/eslint-plugin": "^8.40.0", + "@typescript-eslint/parser": "^8.40.0", "cross-env": "^7.0.3", "esbuild": "0.17.19", "eslint": "^8", @@ -3865,6 +3868,12 @@ "@types/ms": "*" } }, + "node_modules/@types/diff": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@types/diff/-/diff-7.0.2.tgz", + "integrity": "sha512-JSWRMozjFKsGlEjiiKajUjIJVKuKdE3oVy2DNtK+fUo8q82nhFZ2CPQwicAIkXrofahDXrWJ7mjelvZphMS98Q==", + "license": "MIT" + }, "node_modules/@types/dompurify": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/@types/dompurify/-/dompurify-3.2.0.tgz", @@ -6194,6 +6203,15 @@ "integrity": "sha512-gxtyfqMg7GKyhQmb056K7M3xszy/myH8w+B4RT+QXBQsvAOdc3XymqDDPHx1BgPgsdAA5SIifona89YtRATDzw==", "dev": true }, + "node_modules/diff": { + "version": "8.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-8.0.2.tgz", + "integrity": "sha512-sSuxWU5j5SR9QQji/o2qMvqRNYRDOcBTgsJ/DeCf4iSN4gW+gNMXM7wFIP+fdXZxoNiAnHUTGjCr+TSWXdRDKg==", + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/direction": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/direction/-/direction-1.0.4.tgz", diff --git a/gui/package.json b/gui/package.json index ab83efb22fd..6cb45030c9c 100644 --- a/gui/package.json +++ b/gui/package.json @@ -36,10 +36,12 @@ "@tiptap/react": "^2.1.13", "@tiptap/starter-kit": "^2.1.13", "@tiptap/suggestion": "^2.1.13", + "@types/diff": "^7.0.2", "@types/uuid": "^10.0.0", "anser": "^2.3.2", "clsx": "^2.1.1", "core": "file:../core", + "diff": "^8.0.2", "dompurify": "^3.0.6", "downshift": "^7.6.0", "escape-carriage": "^1.3.1", diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx index 1214b9e2425..9b7ba953d2a 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx @@ -11,7 +11,7 @@ interface ApplyActionsProps { applyState?: ApplyState; onClickAccept: () => void; onClickReject: () => void; - onClickApply: () => void; + onClickApply?: () => void; } export function ApplyActions(props: ApplyActionsProps) { diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx index 550d1fd284e..c288c7cd095 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx @@ -1,10 +1,11 @@ import { getLastNPathParts } from "core/util/uri"; +import { MouseEventHandler } from "react"; import FileIcon from "../../FileIcon"; import { ToolTip } from "../../gui/Tooltip"; export interface FileInfoProps { filepath: string; - onClick?: () => void; + onClick?: MouseEventHandler; range?: string; } diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx index e97c13061f6..f9d268ccc3f 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx @@ -96,7 +96,7 @@ export function StepContainerPreToolbar({ toolCallState?.status === "errored" || toolCallState?.status === "done") ? (
{ if (toolCallState.output) { ideMessenger.post("showVirtualFile", { diff --git a/gui/src/pages/gui/ToolCallDiv/EditFile.tsx b/gui/src/pages/gui/ToolCallDiv/EditFile.tsx index dbab7a18df6..3cbcab061c6 100644 --- a/gui/src/pages/gui/ToolCallDiv/EditFile.tsx +++ b/gui/src/pages/gui/ToolCallDiv/EditFile.tsx @@ -17,7 +17,7 @@ export function EditFile(props: EditToolCallProps) { return null; } - const src = `\`\`\`${getMarkdownLanguageTagForFile(props.relativeFilePath)} ${props.relativeFilePath}\n${props.changes}\n\`\`\``; + const src = `\`\`\`${getMarkdownLanguageTagForFile(props.relativeFilePath)} ${props.relativeFilePath}${props.changes ? "\n" + props.changes + "\n" : ""}\`\`\``; return ( ({ + IdeMessengerContext: { + _currentValue: { post: vi.fn() }, + }, +})); + +vi.mock("../../../redux/hooks", () => ({ + useAppSelector: vi.fn(), +})); + +vi.mock("../../../components/ui", () => ({ + useFontSize: () => 14, +})); + +vi.mock("../../../util/clientTools/findAndReplaceUtils", () => ({ + performFindAndReplace: vi.fn(), +})); + +vi.mock("./utils", () => ({ + getStatusIcon: vi.fn(() =>
✓
), +})); + +vi.mock("react", async () => { + const actual = await vi.importActual("react"); + return { + ...actual, + useContext: () => ({ post: vi.fn() }), + }; +}); + +// Import mocked modules +import { useAppSelector } from "../../../redux/hooks"; +import { performFindAndReplace } from "../../../util/clientTools/findAndReplaceUtils"; + +const mockPost = vi.fn(); +const mockUseAppSelector = useAppSelector as any; +const mockPerformFindAndReplace = performFindAndReplace as any; + +describe("FindAndReplaceDisplay", () => { + const defaultProps = { + fileUri: "file:///test/file.ts", + relativeFilePath: "test/file.ts", + editingFileContents: "const old = 'value';\nconst other = 'test';", + edits: [ + { + old_string: "const old = 'value';", + new_string: "const new = 'value';", + replace_all: false, + }, + ] as EditOperation[], + toolCallId: "test-tool-call-id", + historyIndex: 0, + }; + + const mockToolCallState = { + status: "done" as const, + output: null, + }; + + const mockConfig = { + ui: { + showChatScrollbar: true, + codeWrap: false, + }, + }; + + beforeEach(() => { + vi.clearAllMocks(); + + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [] }, + }, + }; + + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return undefined; // No apply state by default + } + if (selector.toString().includes("selectToolCallById")) { + return mockToolCallState; + } + + return selector(mockState); + }); + + mockPerformFindAndReplace.mockImplementation( + (content: string, oldStr: string, newStr: string) => { + return content.replace(oldStr, newStr); + }, + ); + }); + + describe("basic rendering", () => { + it("should render with collapsed state by default", () => { + render(); + + expect(screen.getByText("file.ts")).toBeInTheDocument(); + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + + // Should not show diff content when collapsed + const diffContent = screen.queryByText("-"); + expect(diffContent).not.toBeInTheDocument(); + }); + + it("should display file name from fileUri", () => { + render(); + expect(screen.getByText("file.ts")).toBeInTheDocument(); + }); + + it("should display file name from relativeFilePath when no fileUri", () => { + render( + , + ); + expect(screen.getByText("test.tsx")).toBeInTheDocument(); + }); + + it("should handle missing file paths gracefully", () => { + render( + , + ); + + // Should still render the component structure + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); + }); + + describe("expand/collapse functionality", () => { + it("should expand when clicked", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + // Should show diff content when expanded + expect(screen.getByText("-")).toBeInTheDocument(); + expect(screen.getByText("+")).toBeInTheDocument(); + }); + + it("should show content when tool call status is 'generated'", () => { + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [] }, + }, + }; + + if (selector.toString().includes("selectToolCallById")) { + return { ...mockToolCallState, status: "generated" }; + } + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return undefined; + } + + return selector(mockState); + }); + + render(); + + // Should show diff content without expanding + expect(screen.getByText("-")).toBeInTheDocument(); + expect(screen.getByText("+")).toBeInTheDocument(); + }); + }); + + describe("diff generation", () => { + it("should generate and display diff correctly", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + // Should show removed line + expect(screen.getByText("const old = 'value';")).toBeInTheDocument(); + + // Should show added line + expect(screen.getByText("const new = 'value';")).toBeInTheDocument(); + + // Should show unchanged line + expect(screen.getByText("const other = 'test';")).toBeInTheDocument(); + }); + + it("should handle multiple edits", () => { + const multipleEdits = [ + { + old_string: "const old = 'value';", + new_string: "const new = 'value';", + replace_all: false, + }, + { + old_string: "const other = 'test';", + new_string: "const other = 'updated';", + replace_all: false, + }, + ] as EditOperation[]; + + mockPerformFindAndReplace + .mockReturnValueOnce("const new = 'value';\nconst other = 'test';") + .mockReturnValueOnce("const new = 'value';\nconst other = 'updated';"); + + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledTimes(2); + }); + + it("should handle diff generation errors", () => { + mockPerformFindAndReplace.mockImplementation(() => { + throw new Error("Test error"); + }); + + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(screen.getByText("Error generating diff")).toBeInTheDocument(); + expect(screen.getByText("Test error")).toBeInTheDocument(); + }); + + it("should show 'No changes to display' when diff is empty", () => { + // Mock the function to return the exact same content (no changes) + mockPerformFindAndReplace.mockReturnValue( + defaultProps.editingFileContents, + ); + + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(screen.getByText("No changes to display")).toBeInTheDocument(); + }); + }); + + describe("apply actions", () => { + const mockApplyState: ApplyState = { + streamId: "test-stream-id", + status: "streaming", + numDiffs: 1, + fileContent: "test content", + }; + + beforeEach(() => { + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [mockApplyState] }, + }, + }; + + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return mockApplyState; + } + if (selector.toString().includes("selectToolCallById")) { + return mockToolCallState; + } + + return selector(mockState); + }); + }); + + it("should show apply actions when applyState exists", () => { + render(); + + // ApplyActions component should be rendered (we can test by looking for its structure) + // Since we don't have the exact structure, we test that the container is there + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); + + it("should handle accept action", () => { + render(); + + // This would need more detailed testing if ApplyActions was properly mocked + // For now, we verify the component renders without errors + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); + }); + + describe("content handling", () => { + it("should use editingFileContents when provided", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledWith( + "const old = 'value';\nconst other = 'test';", + "const old = 'value';", + "const new = 'value';", + false, + 0, + ); + }); + + it("should fallback to edit old_strings when no editingFileContents", () => { + render( + , + ); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledWith( + "const old = 'value';", + "const old = 'value';", + "const new = 'value';", + false, + 0, + ); + }); + }); +}); diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx new file mode 100644 index 00000000000..18d995601cd --- /dev/null +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx @@ -0,0 +1,274 @@ +import { ChevronDownIcon } from "@heroicons/react/24/outline"; +import { ApplyState } from "core"; +import { EditOperation } from "core/tools/definitions/multiEdit"; +import { renderContextItems } from "core/util/messageContent"; +import { getLastNPathParts, getUriPathBasename } from "core/util/uri"; +import { diffLines } from "diff"; +import { useContext, useMemo, useState } from "react"; +import { ApplyActions } from "../../../components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions"; +import { FileInfo } from "../../../components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo"; +import { useFontSize } from "../../../components/ui"; +import { IdeMessengerContext } from "../../../context/IdeMessenger"; +import { useAppSelector } from "../../../redux/hooks"; +import { + selectApplyStateByToolCallId, + selectToolCallById, +} from "../../../redux/selectors/selectToolCalls"; +import { performFindAndReplace } from "../../../util/clientTools/findAndReplaceUtils"; +import { getStatusIcon } from "./utils"; + +interface FindAndReplaceDisplayProps { + fileUri?: string; + editingFileContents?: string; + relativeFilePath?: string; + edits: EditOperation[]; + toolCallId: string; + historyIndex: number; +} + +export function FindAndReplaceDisplay({ + fileUri, + relativeFilePath, + editingFileContents, + edits, + toolCallId, + historyIndex, +}: FindAndReplaceDisplayProps) { + const [isExpanded, setIsExpanded] = useState(false); + const ideMessenger = useContext(IdeMessengerContext); + const applyState: ApplyState | undefined = useAppSelector((state) => + selectApplyStateByToolCallId(state, toolCallId), + ); + const fontSize = useFontSize(); + const toolCallState = useAppSelector((state) => + selectToolCallById(state, toolCallId), + ); + const config = useAppSelector((state) => state.config.config); + + const displayName = useMemo(() => { + if (fileUri) { + return getUriPathBasename(fileUri); + } + if (relativeFilePath) { + return getLastNPathParts(relativeFilePath, 1); + } + return ""; + }, [fileUri, relativeFilePath]); + + // Get file content from tool call state instead of reading file + const currentFileContent = useMemo(() => { + if (editingFileContents) { + return editingFileContents; + } + return edits?.map((edit) => edit.old_string ?? "").join("\n"); + }, [editingFileContents, edits]); + + const diffResult = useMemo(() => { + if (!currentFileContent) { + return null; + } + + try { + // Apply all edits sequentially + let newContent = currentFileContent; + for (let i = 0; i < edits.length; i++) { + const { + old_string: oldString, + new_string: newString, + replace_all: replaceAll, + } = edits[i]; + newContent = performFindAndReplace( + newContent, + oldString, + newString, + replaceAll, + i, + ); + } + + // Generate diff between original and final content + const diff = diffLines(currentFileContent, newContent); + return { diff, newContent, error: null }; + } catch (error) { + return { + diff: null, + newContent: null, + error: error instanceof Error ? error.message : "Unknown error", + }; + } + }, [currentFileContent, edits]); + + const statusIcon = useMemo(() => { + const status = toolCallState?.status; + if (status) { + if (status === "canceled" || status === "errored" || status === "done") { + return ( +
{ + if (toolCallState.output) { + e.stopPropagation(); + ideMessenger.post("showVirtualFile", { + name: "Edit output", + content: renderContextItems(toolCallState.output), + }); + } + }} + > + {getStatusIcon(toolCallState.status)} +
+ ); + } + } + }, [toolCallState?.status, toolCallState?.output]); + + // Unified container component that always renders the same structure + const renderContainer = (content: React.ReactNode) => ( +
+
{ + setIsExpanded((prev) => !prev); + }} + > +
+ {statusIcon} + + { + if (!fileUri) { + return; + } + e.stopPropagation(); + ideMessenger.post("openFile", { path: fileUri }); + }} + /> +
+ + {applyState && ( + { + ideMessenger.post(`acceptDiff`, { + filepath: fileUri, + streamId: applyState.streamId, + }); + }} + onClickReject={() => { + ideMessenger.post(`rejectDiff`, { + filepath: fileUri, + streamId: applyState.streamId, + }); + }} + disableManualApply={true} + applyState={applyState} + /> + )} +
+ {toolCallState?.status === "generated" || isExpanded ? content : null} +
+ ); + + if (diffResult?.error) { + return renderContainer( +
+ Error generating diff {diffResult.error} +
, + ); + } + + if ( + !diffResult?.diff || + (diffResult.diff.length === 1 && + !diffResult.diff[0].added && + !diffResult.diff[0].removed) + ) { + return renderContainer( +
No changes to display
, + ); + } + + return renderContainer( +
+ 
+        {diffResult.diff.map((part, index) => {
+          if (part.removed) {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      -
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          } else if (part.added) {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      +
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          } else {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      
+                        {" "}
+                      
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          }
+        })}
+
+
, + ); +} diff --git a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx index 52c03fd85f7..0fcdc0fc16e 100644 --- a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx @@ -1,7 +1,9 @@ import { ToolCallState } from "core"; import { BuiltInToolNames } from "core/tools/builtIn"; +import { EditOperation } from "core/tools/definitions/multiEdit"; import { CreateFile } from "./CreateFile"; import { EditFile } from "./EditFile"; +import { FindAndReplaceDisplay } from "./FindAndReplace"; import { RunTerminalCommand } from "./RunTerminalCommand"; function FunctionSpecificToolCallDiv({ @@ -33,9 +35,9 @@ function FunctionSpecificToolCallDiv({ /> ); case BuiltInToolNames.SearchAndReplaceInFile: - const changes = args.diffs + const changes = args?.diffs ? Array.isArray(args.diffs) - ? args.diffs.join("\n\n---\n\n") + ? args.diffs?.join("\n\n---\n\n") : args.diffs : ""; @@ -49,6 +51,35 @@ function FunctionSpecificToolCallDiv({ historyIndex={historyIndex} /> ); + case BuiltInToolNames.SingleFindAndReplace: + const edits: EditOperation[] = [ + { + old_string: args?.old_string ?? "", + new_string: args?.new_string ?? "", + replace_all: args?.replace_all, + }, + ]; + return ( + + ); + case BuiltInToolNames.MultiEdit: + return ( + + ); case BuiltInToolNames.RunTerminalCommand: return ( ; + }>, + ) => { + const toolCallState = findToolCallById( + state.history, + action.payload.toolCallId, + ); + if (toolCallState) { + toolCallState.parsedArgs = action.payload.newArgs; + } + }, cancelToolCall: ( state, action: PayloadAction<{ @@ -1024,6 +1039,7 @@ export const { acceptToolCall, setToolGenerated, updateToolCallOutput, + setToolCallArgs, setMode, setIsSessionMetadataLoading, setAllSessionMetadata, diff --git a/gui/src/redux/thunks/enhanceParsedArgs.ts b/gui/src/redux/thunks/enhanceParsedArgs.ts new file mode 100644 index 00000000000..75141abb8bc --- /dev/null +++ b/gui/src/redux/thunks/enhanceParsedArgs.ts @@ -0,0 +1,53 @@ +import { BuiltInToolNames } from "core/tools/builtIn"; +import { resolveRelativePathInDir } from "core/util/ideUtils"; +import { getUriPathBasename } from "core/util/uri"; +import { IIdeMessenger } from "../../context/IdeMessenger"; +import { setToolCallArgs } from "../slices/sessionSlice"; +import { AppThunkDispatch } from "../store"; + +export async function enhanceParsedArgs( + ideMessenger: IIdeMessenger, + dispatch: AppThunkDispatch, + toolName: string | undefined, + toolCallId: string, + currentArgs: undefined | Record, +) { + // Add file content to parsedArgs for find/replace tools + let enhancedArgs = { ...currentArgs }; + if ( + (toolName === BuiltInToolNames.SingleFindAndReplace || + toolName === BuiltInToolNames.MultiEdit || + toolName === BuiltInToolNames.SearchAndReplaceInFile) && + currentArgs?.filepath && + !currentArgs?.editingFileContents + ) { + try { + const fileUri = await resolveRelativePathInDir( + currentArgs.filepath, + ideMessenger.ide, + ); + if (!fileUri) { + throw new Error(`File ${currentArgs.filepath} not found`); + } + const baseName = getUriPathBasename(fileUri); + const fileContent = await ideMessenger.ide.readFile(fileUri); + enhancedArgs = { + ...currentArgs, + fileUri, + baseName, + editingFileContents: fileContent, + }; + dispatch( + setToolCallArgs({ + toolCallId, + newArgs: enhancedArgs, + }), + ); + } catch (error) { + // If we can't read the file, let the tool handle the error + console.warn( + `Failed to enhance args: failed to read file ${currentArgs?.filepath}`, + ); + } + } +} diff --git a/gui/src/redux/thunks/handleApplyStateUpdate.test.ts b/gui/src/redux/thunks/handleApplyStateUpdate.test.ts new file mode 100644 index 00000000000..46ae54c35fc --- /dev/null +++ b/gui/src/redux/thunks/handleApplyStateUpdate.test.ts @@ -0,0 +1,762 @@ +import { ApplyState, ApplyToFilePayload, ToolCallState } from "core"; +import { EDIT_MODE_STREAM_ID } from "core/edit/constants"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { logAgentModeEditOutcome } from "../../util/editOutcomeLogger"; +import { + selectApplyStateByToolCallId, + selectToolCallById, +} from "../selectors/selectToolCalls"; +import { updateEditStateApplyState } from "../slices/editState"; +import { + acceptToolCall, + errorToolCall, + updateApplyState, + updateToolCallOutput, +} from "../slices/sessionSlice"; +import { findToolCallById, logToolUsage } from "../util"; +import { exitEdit } from "./edit"; +import { + applyForEditTool, + handleApplyStateUpdate, +} from "./handleApplyStateUpdate"; +import { streamResponseAfterToolCall } from "./streamResponseAfterToolCall"; + +// Mock dependencies +vi.mock("../../util/editOutcomeLogger", () => ({ + logAgentModeEditOutcome: vi.fn(), +})); + +vi.mock("../selectors/selectToolCalls", () => ({ + selectApplyStateByToolCallId: vi.fn(), + selectToolCallById: vi.fn(), +})); + +vi.mock("../slices/editState", () => ({ + updateEditStateApplyState: vi.fn(() => ({ + type: "editState/updateApplyState", + })), +})); + +vi.mock("../slices/sessionSlice", () => ({ + acceptToolCall: vi.fn(() => ({ type: "session/acceptToolCall" })), + errorToolCall: vi.fn(() => ({ type: "session/errorToolCall" })), + updateApplyState: vi.fn(() => ({ type: "session/updateApplyState" })), + updateToolCallOutput: vi.fn(() => ({ type: "session/updateToolCallOutput" })), +})); + +vi.mock("../util", () => ({ + findToolCallById: vi.fn(), + logToolUsage: vi.fn(), +})); + +vi.mock("./edit", () => ({ + exitEdit: vi.fn(() => ({ type: "edit/exitEdit" })), +})); + +vi.mock("./streamResponseAfterToolCall", () => ({ + streamResponseAfterToolCall: vi.fn(() => ({ + type: "session/streamResponseAfterToolCall", + })), +})); + +const UNUSED_TOOL_CALL_PARAMS = { + toolCall: { + id: "name", + function: { + name: "unused", + arguments: "unused", + }, + type: "function" as const, + }, + parsedArgs: {}, +}; + +describe("handleApplyStateUpdate", () => { + let mockDispatch: any; + let mockGetState: any; + let mockExtra: any; + + beforeEach(() => { + vi.clearAllMocks(); + mockDispatch = vi.fn(); + mockGetState = vi.fn(); + mockExtra = { + ideMessenger: { + post: vi.fn(), + request: vi.fn(), + }, + }; + }); + + describe("edit mode handling", () => { + it("should handle edit mode apply state updates", async () => { + const applyState: ApplyState = { + streamId: EDIT_MODE_STREAM_ID, + toolCallId: "test-tool-call", + status: "streaming", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(updateEditStateApplyState).toHaveBeenCalledWith(applyState); + expect(mockDispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: "editState/updateApplyState" }), + ); + }); + + it("should exit edit mode when status is closed", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "done", + ...UNUSED_TOOL_CALL_PARAMS, + }; + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + + mockGetState.mockReturnValue({ + session: { history: [] }, + }); + + const applyState: ApplyState = { + streamId: EDIT_MODE_STREAM_ID, + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logToolUsage).toHaveBeenCalledWith( + toolCallState, + true, + true, + mockExtra.ideMessenger, + ); + expect(exitEdit).toHaveBeenCalledWith({}); + expect(mockDispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: "edit/exitEdit" }), + ); + }); + }); + + describe("chat/agent mode handling", () => { + it("should handle non-edit mode apply state updates", async () => { + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "streaming", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(updateApplyState).toHaveBeenCalledWith(applyState); + expect(mockDispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: "session/updateApplyState" }), + ); + }); + + it("should auto-accept diffs when configured and status is done", async () => { + mockGetState.mockReturnValue({ + config: { + config: { + ui: { + autoAcceptEditToolDiffs: true, + }, + }, + }, + session: { history: [] }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "done", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(mockExtra.ideMessenger.post).toHaveBeenCalledWith("acceptDiff", { + streamId: "chat-stream", + filepath: "test.txt", + }); + }); + + it("should not auto-accept when config is disabled", async () => { + mockGetState.mockReturnValue({ + config: { + config: { + ui: { + autoAcceptEditToolDiffs: false, + }, + }, + }, + session: { history: [] }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "done", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(mockExtra.ideMessenger.post).not.toHaveBeenCalled(); + }); + }); + + describe("closed status handling", () => { + it("should handle accepted tool call closure", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "done", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const newApplyState = { streamId: "chat-stream" }; + + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [newApplyState], + }, + }, + config: { config: {} }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logToolUsage).toHaveBeenCalledWith( + toolCallState, + true, + true, + mockExtra.ideMessenger, + ); + expect(logAgentModeEditOutcome).toHaveBeenCalledWith( + [], + {}, + toolCallState, + newApplyState, + true, + mockExtra.ideMessenger, + ); + expect(acceptToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + expect(streamResponseAfterToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + }); + + it("should handle canceled tool call closure", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "canceled", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const newApplyState = { streamId: "chat-stream" }; + + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [newApplyState], + }, + }, + config: { config: {} }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logToolUsage).toHaveBeenCalledWith( + toolCallState, + false, + true, + mockExtra.ideMessenger, + ); + expect(logAgentModeEditOutcome).toHaveBeenCalledWith( + [], + {}, + toolCallState, + newApplyState, + false, + mockExtra.ideMessenger, + ); + expect(acceptToolCall).not.toHaveBeenCalled(); + expect(streamResponseAfterToolCall).not.toHaveBeenCalled(); + }); + + it("should handle errored tool call closure", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "errored", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const newApplyState = { streamId: "chat-stream" }; + + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [newApplyState], + }, + }, + config: { config: {} }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(acceptToolCall).not.toHaveBeenCalled(); + expect(streamResponseAfterToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + }); + + it("should handle closure when no tool call found", async () => { + vi.mocked(findToolCallById).mockReturnValue(undefined); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [], + }, + }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logToolUsage).not.toHaveBeenCalled(); + expect(acceptToolCall).not.toHaveBeenCalled(); + expect(streamResponseAfterToolCall).not.toHaveBeenCalled(); + }); + + it("should handle closure when no apply state found", async () => { + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "done", + ...UNUSED_TOOL_CALL_PARAMS, + }; + + vi.mocked(findToolCallById).mockReturnValue(toolCallState); + mockGetState.mockReturnValue({ + session: { + history: [], + codeBlockApplyStates: { + states: [], + }, + }, + config: { config: {} }, + }); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status: "closed", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(logAgentModeEditOutcome).not.toHaveBeenCalled(); + expect(acceptToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + }); + }); + + describe("edge cases", () => { + it("should handle apply state without toolCallId", async () => { + const applyState: ApplyState = { + streamId: "chat-stream", + status: "streaming", + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(updateApplyState).toHaveBeenCalledWith(applyState); + expect(acceptToolCall).not.toHaveBeenCalled(); + expect(streamResponseAfterToolCall).not.toHaveBeenCalled(); + }); + + it("should handle different status values", async () => { + const statusValues: ApplyState["status"][] = [ + "not-started", + "streaming", + "done", + "closed", + ]; + + for (const status of statusValues) { + vi.clearAllMocks(); + + const applyState: ApplyState = { + streamId: "chat-stream", + toolCallId: "test-tool-call", + status, + filepath: "test.txt", + numDiffs: 1, + }; + + const thunk = handleApplyStateUpdate(applyState); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(updateApplyState).toHaveBeenCalledWith(applyState); + } + }); + }); +}); + +describe("applyForEditTool", () => { + let mockDispatch: any; + let mockGetState: any; + let mockExtra: any; + + beforeEach(() => { + vi.clearAllMocks(); + mockDispatch = vi.fn(); + mockGetState = vi.fn(); + mockExtra = { + ideMessenger: { + request: vi.fn(), + }, + }; + }); + + describe("successful application", () => { + it("should successfully apply changes to file", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + mockExtra.ideMessenger.request.mockResolvedValue({ status: "success" }); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(updateApplyState).toHaveBeenCalledWith({ + streamId: "test-stream", + toolCallId: "test-tool-call", + status: "not-started", + }); + expect(mockDispatch).toHaveBeenCalledWith( + expect.objectContaining({ type: "session/updateApplyState" }), + ); + expect(mockExtra.ideMessenger.request).toHaveBeenCalledWith( + "applyToFile", + payload, + ); + }); + }); + + describe("error handling", () => { + it("should handle IDE messenger request failure", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "calling", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const applyState: ApplyState = { + status: "streaming", + streamId: "test-stream", + }; + + vi.mocked(selectToolCallById).mockReturnValue(toolCallState); + vi.mocked(selectApplyStateByToolCallId).mockReturnValue(applyState); + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockRejectedValue( + new Error("Request failed"), + ); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + expect(updateToolCallOutput).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + contextItems: [ + { + icon: "problems", + name: "Apply Error", + description: "Failed to apply changes", + content: `Error editing file: failed to apply changes to file.\n\nPlease try again with correct args or notify the user and request further instructions.`, + hidden: false, + }, + ], + }); + }); + + it("should handle IDE messenger error response", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "calling", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const applyState: ApplyState = { + status: "streaming", + streamId: "test-stream", + }; + + vi.mocked(selectToolCallById).mockReturnValue(toolCallState); + vi.mocked(selectApplyStateByToolCallId).mockReturnValue(applyState); + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockResolvedValue({ status: "error" }); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + }); + expect(updateToolCallOutput).toHaveBeenCalledWith({ + toolCallId: "test-tool-call", + contextItems: [ + { + icon: "problems", + name: "Apply Error", + description: "Failed to apply changes", + content: `Error editing file: failed to apply changes to file.\n\nPlease try again with correct args or notify the user and request further instructions.`, + hidden: false, + }, + ], + }); + }); + + it("should not error if tool call is not in calling state", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "done", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const applyState: ApplyState = { + status: "streaming", + streamId: "test-stream", + }; + + vi.mocked(selectToolCallById).mockReturnValue(toolCallState); + vi.mocked(selectApplyStateByToolCallId).mockReturnValue(applyState); + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockRejectedValue( + new Error("Request failed"), + ); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).not.toHaveBeenCalled(); + expect(updateToolCallOutput).not.toHaveBeenCalled(); + }); + + it("should not error if apply state is closed", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + const toolCallState: ToolCallState = { + toolCallId: "test-tool-call", + status: "calling", + ...UNUSED_TOOL_CALL_PARAMS, + }; + const applyState: ApplyState = { + status: "closed", + streamId: "test-stream", + }; // Already closed + + vi.mocked(selectToolCallById).mockReturnValue(toolCallState); + vi.mocked(selectApplyStateByToolCallId).mockReturnValue(applyState); + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockRejectedValue( + new Error("Request failed"), + ); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).not.toHaveBeenCalled(); + expect(updateToolCallOutput).not.toHaveBeenCalled(); + }); + + it("should not error if tool call state is not found", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + vi.mocked(selectToolCallById).mockReturnValue(undefined); // Tool call not found + vi.mocked(selectApplyStateByToolCallId).mockReturnValue({ + status: "streaming", + streamId: "test-stream", + }); + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockRejectedValue( + new Error("Request failed"), + ); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).not.toHaveBeenCalled(); + expect(updateToolCallOutput).not.toHaveBeenCalled(); + }); + + it("should not error if apply state is not found", async () => { + const payload: ApplyToFilePayload & { toolCallId: string } = { + toolCallId: "test-tool-call", + streamId: "test-stream", + filepath: "test.txt", + text: "new content", + isSearchAndReplace: true, + }; + + vi.mocked(selectApplyStateByToolCallId).mockReturnValue(undefined); // Apply state not found + mockGetState.mockReturnValue({}); + + mockExtra.ideMessenger.request.mockRejectedValue( + new Error("Request failed"), + ); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(errorToolCall).not.toHaveBeenCalled(); + expect(updateToolCallOutput).not.toHaveBeenCalled(); + }); + }); + + describe("edge cases", () => { + it("should handle different payload types", async () => { + const payloads: (ApplyToFilePayload & { toolCallId: string })[] = [ + { + toolCallId: "test-1", + streamId: "stream-1", + filepath: "file1.txt", + text: "content 1", + isSearchAndReplace: false, + }, + { + toolCallId: "test-2", + streamId: "stream-2", + filepath: "file2.js", + text: "content 2", + isSearchAndReplace: true, + }, + ]; + + for (const payload of payloads) { + vi.clearAllMocks(); + mockExtra.ideMessenger.request.mockResolvedValue({ status: "success" }); + + const thunk = applyForEditTool(payload); + await thunk(mockDispatch, mockGetState, mockExtra); + + expect(mockExtra.ideMessenger.request).toHaveBeenCalledWith( + "applyToFile", + payload, + ); + expect(updateApplyState).toHaveBeenCalledWith({ + streamId: payload.streamId, + toolCallId: payload.toolCallId, + status: "not-started", + }); + } + }); + }); +}); diff --git a/gui/src/redux/thunks/handleApplyStateUpdate.ts b/gui/src/redux/thunks/handleApplyStateUpdate.ts index 1a56fe04be1..25532e75079 100644 --- a/gui/src/redux/thunks/handleApplyStateUpdate.ts +++ b/gui/src/redux/thunks/handleApplyStateUpdate.ts @@ -1,5 +1,5 @@ import { createAsyncThunk } from "@reduxjs/toolkit"; -import { ApplyState } from "core"; +import { ApplyState, ApplyToFilePayload } from "core"; import { EDIT_MODE_STREAM_ID } from "core/edit/constants"; import { logAgentModeEditOutcome } from "../../util/editOutcomeLogger"; import { @@ -105,13 +105,30 @@ export const handleApplyStateUpdate = createAsyncThunk< }, ); -export const handleEditToolApplyError = createAsyncThunk< +export const applyForEditTool = createAsyncThunk< void, - { toolCallId: string }, + ApplyToFilePayload & { toolCallId: string }, ThunkApiType ->( - "apply/handleEditError", - async ({ toolCallId }, { dispatch, getState, extra }) => { +>("apply/editTool", async (payload, { dispatch, getState, extra }) => { + const { toolCallId, streamId } = payload; + dispatch( + updateApplyState({ + streamId, + toolCallId, + status: "not-started", + }), + ); + + let didError = false; + try { + const response = await extra.ideMessenger.request("applyToFile", payload); + if (response.status === "error") { + didError = true; + } + } catch (e) { + didError = true; + } + if (didError) { const state = getState(); const toolCallState = selectToolCallById(state, toolCallId); @@ -149,5 +166,5 @@ export const handleEditToolApplyError = createAsyncThunk< }), ); } - }, -); + } +}); diff --git a/gui/src/redux/thunks/streamNormalInput.ts b/gui/src/redux/thunks/streamNormalInput.ts index 218883cb919..dbd322357d8 100644 --- a/gui/src/redux/thunks/streamNormalInput.ts +++ b/gui/src/redux/thunks/streamNormalInput.ts @@ -1,8 +1,7 @@ import { createAsyncThunk, unwrapResult } from "@reduxjs/toolkit"; -import { LLMFullCompletionOptions, ModelDescription, Tool } from "core"; +import { LLMFullCompletionOptions, Tool } from "core"; import { getRuleId } from "core/llm/rules/getSystemMessageWithRules"; import { ToCoreProtocol } from "core/protocol"; -import { BuiltInToolNames } from "core/tools/builtIn"; import { selectActiveTools } from "../selectors/selectActiveTools"; import { selectUseSystemMessageTools } from "../selectors/selectUseSystemMessageTools"; import { selectSelectedChatModel } from "../slices/configSlice"; @@ -29,6 +28,7 @@ import { selectCurrentToolCalls } from "../selectors/selectToolCalls"; import { DEFAULT_TOOL_SETTING } from "../slices/uiSlice"; import { getBaseSystemMessage } from "../util/getBaseSystemMessage"; import { callToolById } from "./callToolById"; +import { enhanceParsedArgs } from "./enhanceParsedArgs"; /** * Handles the execution of tool calls that may be automatically accepted. * Sets all tools as generated first, then executes auto-approved tool calls. @@ -86,49 +86,6 @@ async function handleToolCallExecution( } } -/** - * Filters tools based on the selected model's capabilities. - * Returns either the edit file tool or search and replace tool, but not both. - */ -function filterToolsForModel( - tools: Tool[], - selectedModel: ModelDescription, -): Tool[] { - const editFileTool = tools.find( - (tool) => tool.function.name === BuiltInToolNames.EditExistingFile, - ); - const searchAndReplaceTool = tools.find( - (tool) => tool.function.name === BuiltInToolNames.SearchAndReplaceInFile, - ); - - // If we don't have both tools, return tools as-is - if (!editFileTool || !searchAndReplaceTool) { - return tools; - } - - // Determine which tool to use based on the model - const shouldUseFindReplace = shouldUseFindReplaceEdits(selectedModel); - - // Filter out the unwanted tool - return tools.filter((tool) => { - if (tool.function.name === BuiltInToolNames.EditExistingFile) { - return !shouldUseFindReplace; - } - if (tool.function.name === BuiltInToolNames.SearchAndReplaceInFile) { - return shouldUseFindReplace; - } - return true; - }); -} - -/** - * Determines whether to use search and replace tool instead of edit file - * Right now we only know that this is reliable with Claude models - */ -function shouldUseFindReplaceEdits({ model }: ModelDescription): boolean { - return model.includes("claude") || model.includes("gpt-5"); -} - export const streamNormalInput = createAsyncThunk< void, { @@ -146,8 +103,8 @@ export const streamNormalInput = createAsyncThunk< } // Get tools and filter them based on the selected model - const allActiveTools = selectActiveTools(state); - const activeTools = filterToolsForModel(allActiveTools, selectedChatModel); + const activeTools = selectActiveTools(state); + const supportsNativeTools = modelSupportsNativeTools(selectedChatModel); // Use the centralized selector to determine if system message tools should be used @@ -297,6 +254,19 @@ export const streamNormalInput = createAsyncThunk< const newState = getState(); const toolSettings = newState.ui.toolSettings; const allToolCallStates = selectCurrentToolCalls(newState); + + await Promise.all( + allToolCallStates.map((tcState) => + enhanceParsedArgs( + extra.ideMessenger, + dispatch, + tcState?.toolCall.function.name, + tcState.toolCallId, + tcState.parsedArgs, + ), + ), + ); + const generatingToolCalls = allToolCallStates.filter( (toolCallState) => toolCallState.status === "generating", ); diff --git a/gui/src/util/clientTools/callClientTool.ts b/gui/src/util/clientTools/callClientTool.ts index b5ac685d305..755be6e7345 100644 --- a/gui/src/util/clientTools/callClientTool.ts +++ b/gui/src/util/clientTools/callClientTool.ts @@ -3,7 +3,9 @@ import { BuiltInToolNames } from "core/tools/builtIn"; import { IIdeMessenger } from "../../context/IdeMessenger"; import { AppThunkDispatch, RootState } from "../../redux/store"; import { editToolImpl } from "./editImpl"; +import { multiEditImpl } from "./multiEditImpl"; import { searchReplaceToolImpl } from "./searchReplaceImpl"; +import { singleFindAndReplaceImpl } from "./singleFindAndReplaceImpl"; export interface ClientToolExtras { getState: () => RootState; @@ -40,6 +42,16 @@ export async function callClientTool( case BuiltInToolNames.SearchAndReplaceInFile: output = await searchReplaceToolImpl(parsedArgs, toolCall.id, extras); break; + case BuiltInToolNames.SingleFindAndReplace: + output = await singleFindAndReplaceImpl( + parsedArgs, + toolCall.id, + extras, + ); + break; + case BuiltInToolNames.MultiEdit: + output = await multiEditImpl(parsedArgs, toolCall.id, extras); + break; default: throw new Error(`Invalid client tool name ${toolCall.function.name}`); } diff --git a/gui/src/util/clientTools/editImpl.ts b/gui/src/util/clientTools/editImpl.ts index edd49dd0617..4dec61a65c3 100644 --- a/gui/src/util/clientTools/editImpl.ts +++ b/gui/src/util/clientTools/editImpl.ts @@ -1,7 +1,6 @@ import { resolveRelativePathInDir } from "core/util/ideUtils"; import { v4 as uuid } from "uuid"; -import { updateApplyState } from "../../redux/slices/sessionSlice"; -import { handleEditToolApplyError } from "../../redux/thunks/handleApplyStateUpdate"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; import { ClientToolImpl } from "./callClientTool"; export const editToolImpl: ClientToolImpl = async ( args, @@ -21,29 +20,15 @@ export const editToolImpl: ClientToolImpl = async ( throw new Error(`${args.filepath} does not exist`); } const streamId = uuid(); - extras.dispatch( - updateApplyState({ - streamId, - toolCallId, - status: "not-started", - }), - ); - void extras.ideMessenger - .request("applyToFile", { + void extras.dispatch( + applyForEditTool({ streamId, text: args.changes, toolCallId, filepath: firstUriMatch, - }) - .then((res) => { - if (res.status === "error") { - void extras.dispatch( - handleEditToolApplyError({ - toolCallId, - }), - ); - } - }); + }), + ); + return { respondImmediately: false, output: undefined, // No immediate output. diff --git a/gui/src/util/clientTools/findAndReplaceUtils.test.ts b/gui/src/util/clientTools/findAndReplaceUtils.test.ts new file mode 100644 index 00000000000..68ab333b9a3 --- /dev/null +++ b/gui/src/util/clientTools/findAndReplaceUtils.test.ts @@ -0,0 +1,456 @@ +import { EditOperation } from "core/tools/definitions/multiEdit"; +import { describe, expect, it } from "vitest"; +import { + EMPTY_NON_FIRST_EDIT_MESSAGE, + FOUND_MULTIPLE_FIND_STRINGS_ERROR, + performFindAndReplace, + validateCreatingForMultiEdit, + validateSingleEdit, +} from "./findAndReplaceUtils"; + +describe("performFindAndReplace", () => { + describe("normal cases", () => { + it("should replace single occurrence by default", () => { + const content = "Hello world\nThis is a test file\nGoodbye"; + const result = performFindAndReplace(content, "Hello", "Hi"); + + expect(result).toBe("Hi world\nThis is a test file\nGoodbye"); + }); + + it("should replace single occurrence when replaceAll is false", () => { + const content = "Hello world\nHello again"; + const result = performFindAndReplace( + content, + "Hello world", + "Hi world", + false, + ); + + expect(result).toBe("Hi world\nHello again"); + }); + + it("should replace all occurrences when replaceAll is true", () => { + const content = "Hello world\nHello again\nHello there"; + const result = performFindAndReplace(content, "Hello", "Hi", true); + + expect(result).toBe("Hi world\nHi again\nHi there"); + }); + + it("should handle empty new string (deletion)", () => { + const content = "Hello world"; + const result = performFindAndReplace(content, "Hello ", ""); + + expect(result).toBe("world"); + }); + + it("should handle empty old string replacement (insertion at start)", () => { + const content = ""; + const result = performFindAndReplace(content, "", "Hello world"); + + expect(result).toBe("Hello world"); + }); + + it("should preserve whitespace and indentation", () => { + const content = + "function test() {\n const value = 'old';\n return value;\n}"; + const result = performFindAndReplace( + content, + " const value = 'old';", + " const value = 'new';", + ); + + expect(result).toBe( + "function test() {\n const value = 'new';\n return value;\n}", + ); + }); + + it("should handle multiline strings", () => { + const content = "line1\nline2\nline3"; + const result = performFindAndReplace( + content, + "line1\nline2", + "newline1\nnewline2", + ); + + expect(result).toBe("newline1\nnewline2\nline3"); + }); + }); + + describe("special character handling", () => { + it("should handle regex special characters in old string", () => { + const content = "const regex = /[a-z]+/g;"; + const result = performFindAndReplace(content, "/[a-z]+/g", "/[A-Z]+/g"); + + expect(result).toBe("const regex = /[A-Z]+/g;"); + }); + + it("should handle dollar signs in strings", () => { + const content = 'const text = "Hello $world"'; + const result = performFindAndReplace( + content, + '"Hello $world"', + '"Hi $universe"', + ); + + expect(result).toBe('const text = "Hi $universe"'); + }); + + it("should handle backslashes", () => { + const content = 'const path = "C:\\Users\\test"'; + const result = performFindAndReplace( + content, + "C:\\Users\\test", + "D:\\Users\\test", + ); + + expect(result).toBe('const path = "D:\\Users\\test"'); + }); + + it("should handle parentheses", () => { + const content = "function test() { return 'old'; }"; + const result = performFindAndReplace( + content, + "() { return 'old'; }", + "() { return 'new'; }", + ); + + expect(result).toBe("function test() { return 'new'; }"); + }); + + it("should handle square brackets", () => { + const content = "const arr = [1, 2, 3];"; + const result = performFindAndReplace(content, "[1, 2, 3]", "[4, 5, 6]"); + + expect(result).toBe("const arr = [4, 5, 6];"); + }); + }); + + describe("replaceAll behavior", () => { + it("should replace all occurrences including partial matches", () => { + const content = "test testing tested test"; + const result = performFindAndReplace(content, "test", "exam", true); + + expect(result).toBe("exam examing examed exam"); + }); + + it("should replace all occurrences with empty string", () => { + const content = "remove this remove that remove everything"; + const result = performFindAndReplace(content, "remove ", "", true); + + expect(result).toBe("this that everything"); + }); + + it("should handle overlapping patterns correctly", () => { + const content = "aaaa"; + const result = performFindAndReplace(content, "aa", "b", true); + + expect(result).toBe("bb"); + }); + }); + + describe("error cases", () => { + it("should throw error when string not found", () => { + const content = "Hello world"; + + expect(() => { + performFindAndReplace(content, "xyz", "abc"); + }).toThrow('string not found in file: "xyz"'); + }); + + it("should throw error with edit context when string not found", () => { + const content = "Hello world"; + + expect(() => { + performFindAndReplace(content, "xyz", "abc", false, 2); + }).toThrow('edit at index 2: string not found in file: "xyz"'); + }); + + it("should throw error when multiple occurrences exist and replaceAll is false", () => { + const content = "Hello world\nHello again\nHello there"; + + expect(() => { + performFindAndReplace(content, "Hello", "Hi", false); + }).toThrow( + `String "Hello" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`, + ); + }); + + it("should throw error with edit context for multiple occurrences", () => { + const content = "test test test"; + + expect(() => { + performFindAndReplace(content, "test", "exam", false, 1); + }).toThrow( + `edit at index 1: String "test" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`, + ); + }); + }); + + describe("edge cases", () => { + it("should handle single character replacement", () => { + const content = "a b c"; + const result = performFindAndReplace(content, "b", "x"); + + expect(result).toBe("a x c"); + }); + + it("should handle very long strings", () => { + const longString = "a".repeat(10000); + const content = `start${longString}end`; + const result = performFindAndReplace(content, longString, "short"); + + expect(result).toBe("startshortend"); + }); + + it("should handle strings at beginning of content", () => { + const content = "start middle end"; + const result = performFindAndReplace(content, "start", "begin"); + + expect(result).toBe("begin middle end"); + }); + + it("should handle strings at end of content", () => { + const content = "start middle end"; + const result = performFindAndReplace(content, "end", "finish"); + + expect(result).toBe("start middle finish"); + }); + + it("should handle entire content replacement", () => { + const content = "replace everything"; + const result = performFindAndReplace( + content, + "replace everything", + "new content", + ); + + expect(result).toBe("new content"); + }); + }); +}); + +describe("validateSingleEdit", () => { + describe("valid inputs", () => { + it("should not throw for valid non-empty strings", () => { + expect(() => { + validateSingleEdit("old", "new"); + }).not.toThrow(); + }); + + it("should allow empty old_string for file creation", () => { + expect(() => { + validateSingleEdit("", "new content"); + }).not.toThrow(); + }); + + it("should allow empty new_string for deletion", () => { + expect(() => { + validateSingleEdit("delete me", ""); + }).not.toThrow(); + }); + + it("should allow multiline strings", () => { + expect(() => { + validateSingleEdit("line1\nline2", "newline1\nnewline2"); + }).not.toThrow(); + }); + + it("should allow special characters", () => { + expect(() => { + validateSingleEdit("/[a-z]+/g", "/[A-Z]+/g"); + }).not.toThrow(); + }); + }); + + describe("invalid inputs", () => { + it("should throw error when old_string is null", () => { + expect(() => { + validateSingleEdit(null as any, "new"); + }).toThrow("old_string is required"); + }); + + it("should throw error when old_string is undefined", () => { + expect(() => { + validateSingleEdit(undefined as any, "new"); + }).toThrow("old_string is required"); + }); + + it("should throw error when new_string is undefined", () => { + expect(() => { + validateSingleEdit("old", undefined as any); + }).toThrow("new_string is required"); + }); + + it("should throw error when old_string and new_string are identical", () => { + expect(() => { + validateSingleEdit("same", "same"); + }).toThrow("old_string and new_string must be different"); + }); + + it("should throw error when both are empty strings", () => { + expect(() => { + validateSingleEdit("", ""); + }).toThrow("old_string and new_string must be different"); + }); + }); + + describe("error messages with context", () => { + it("should include edit number in error messages when index provided", () => { + expect(() => { + validateSingleEdit(null as any, "new", 2); + }).toThrow("edit at index 2: old_string is required"); + }); + + it("should include edit number for new_string errors", () => { + expect(() => { + validateSingleEdit("old", undefined as any, 0); + }).toThrow("edit at index 0: new_string is required"); + }); + + it("should include edit number for identical strings error", () => { + expect(() => { + validateSingleEdit("same", "same", 4); + }).toThrow( + "edit at index 4: old_string and new_string must be different", + ); + }); + + it("should not include context when index not provided", () => { + expect(() => { + validateSingleEdit("same", "same"); + }).toThrow("old_string and new_string must be different"); + }); + }); +}); + +describe("validateCreatingForMultiEdit", () => { + describe("single edit scenarios", () => { + it("should return true for creating with single edit", () => { + const edits: EditOperation[] = [ + { old_string: "", new_string: "new file content" }, + ]; + + const isCreating = validateCreatingForMultiEdit(edits); + expect(isCreating).toBe(true); + }); + + it("should return false for editing with single edit", () => { + const edits: EditOperation[] = [{ old_string: "old", new_string: "new" }]; + + const isCreating = validateCreatingForMultiEdit(edits); + expect(isCreating).toBe(false); + }); + }); + + describe("multiple edit scenarios - editing existing file", () => { + it("should return false for multiple edits on existing file", () => { + const edits: EditOperation[] = [ + { old_string: "old1", new_string: "new1" }, + { old_string: "old2", new_string: "new2" }, + { old_string: "old3", new_string: "new3" }, + ]; + + const isCreating = validateCreatingForMultiEdit(edits); + expect(isCreating).toBe(false); + }); + + it("should allow multiple valid edits on existing file", () => { + const edits: EditOperation[] = [ + { old_string: "import old", new_string: "import new" }, + { old_string: "function old()", new_string: "function new()" }, + { old_string: "return old;", new_string: "return new;" }, + ]; + + expect(() => validateCreatingForMultiEdit(edits)).not.toThrow(); + expect(validateCreatingForMultiEdit(edits)).toBe(false); + }); + }); + + describe("error cases", () => { + it("should throw error when trying to make subsequent edits on file creation", () => { + const edits: EditOperation[] = [ + { old_string: "", new_string: "new file content" }, + { old_string: "old", new_string: "new" }, + ]; + + expect(() => { + validateCreatingForMultiEdit(edits); + }).toThrow("cannot make subsequent edits on a file you are creating"); + }); + + it("should throw error when empty old_string appears in non-first edit", () => { + const edits: EditOperation[] = [ + { old_string: "old1", new_string: "new1" }, + { old_string: "", new_string: "new2" }, + ]; + + expect(() => { + validateCreatingForMultiEdit(edits); + }).toThrow(`edit at index 1: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`); + }); + + it("should throw error with correct edit number for empty old_string", () => { + const edits: EditOperation[] = [ + { old_string: "old1", new_string: "new1" }, + { old_string: "old2", new_string: "new2" }, + { old_string: "", new_string: "new3" }, + ]; + + expect(() => { + validateCreatingForMultiEdit(edits); + }).toThrow(`edit at index 2: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`); + }); + + it("should throw error for multiple empty old_strings in subsequent edits", () => { + const edits: EditOperation[] = [ + { old_string: "old1", new_string: "new1" }, + { old_string: "", new_string: "new2" }, + { old_string: "", new_string: "new3" }, + ]; + + expect(() => { + validateCreatingForMultiEdit(edits); + }).toThrow(`edit at index 1: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`); + }); + }); + + describe("edge cases", () => { + it("should handle empty edits array gracefully", () => { + const edits: EditOperation[] = []; + + // This should not throw, but would likely cause issues elsewhere + // The function doesn't explicitly handle empty arrays + expect(() => { + validateCreatingForMultiEdit(edits); + }).toThrow(); // Will throw because edits[0] is undefined + }); + + it("should handle complex edit operations", () => { + const edits: EditOperation[] = [ + { + old_string: "function oldFunction() {\n return 'old';\n}", + new_string: "function newFunction() {\n return 'new';\n}", + replace_all: false, + }, + { + old_string: "const OLD_CONSTANT", + new_string: "const NEW_CONSTANT", + replace_all: true, + }, + ]; + + expect(() => validateCreatingForMultiEdit(edits)).not.toThrow(); + expect(validateCreatingForMultiEdit(edits)).toBe(false); + }); + + it("should handle edits with replace_all flags", () => { + const edits: EditOperation[] = [ + { old_string: "old", new_string: "new", replace_all: true }, + { old_string: "test", new_string: "exam", replace_all: false }, + ]; + + expect(() => validateCreatingForMultiEdit(edits)).not.toThrow(); + expect(validateCreatingForMultiEdit(edits)).toBe(false); + }); + }); +}); diff --git a/gui/src/util/clientTools/findAndReplaceUtils.ts b/gui/src/util/clientTools/findAndReplaceUtils.ts new file mode 100644 index 00000000000..6828e1695b4 --- /dev/null +++ b/gui/src/util/clientTools/findAndReplaceUtils.ts @@ -0,0 +1,97 @@ +import { EditOperation } from "core/tools/definitions/multiEdit"; + +export const FOUND_MULTIPLE_FIND_STRINGS_ERROR = + "Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences."; + +/** + * Performs a find and replace operation on text content with proper handling of special characters + */ +export function performFindAndReplace( + content: string, + oldString: string, + newString: string, + replaceAll: boolean = false, + index?: number, // For error messages +): string { + const errorContext = index !== undefined ? `edit at index ${index}: ` : ""; + // Check if old_string exists in current content + if (!content.includes(oldString)) { + throw new Error(`${errorContext}string not found in file: "${oldString}"`); + } + + if (replaceAll) { + // Replace all occurrences using replaceAll for proper handling of special characters + return content.replaceAll(oldString, newString); + } else { + // Handle empty oldString case (insertion) + if (oldString === "") { + return newString + content; + } + + // Count occurrences using indexOf for proper handling of special characters + let count = 0; + let searchIndex = content.indexOf(oldString); + while (searchIndex !== -1) { + count++; + searchIndex = content.indexOf(oldString, searchIndex + 1); + } + + if (count > 1) { + throw new Error( + `${errorContext}String "${oldString}" appears ${count} times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`, + ); + } + + // Replace only the first occurrence + const firstIndex = content.indexOf(oldString); + return ( + content.substring(0, firstIndex) + + newString + + content.substring(firstIndex + oldString.length) + ); + } +} + +/** + * Validates a single edit operation + */ +export function validateSingleEdit( + oldString: string, + newString: string, + index?: number, +): void { + const context = index !== undefined ? `edit at index ${index}: ` : ""; + + if (!oldString && oldString !== "") { + throw new Error(`${context}old_string is required`); + } + if (newString === undefined) { + throw new Error(`${context}new_string is required`); + } + if (oldString === newString) { + throw new Error(`${context}old_string and new_string must be different`); + } +} + +export const EMPTY_NON_FIRST_EDIT_MESSAGE = + "contains empty old_string. Only the first edit can contain an empty old_string, which is only used for file creation."; +export function validateCreatingForMultiEdit(edits: EditOperation[]) { + const isCreating = edits[0].old_string === ""; + if (edits.length > 1) { + if (isCreating) { + throw new Error( + "cannot make subsequent edits on a file you are creating", + ); + } else { + for (let i = 1; i < edits.length; i++) { + if (edits[i].old_string === "") { + throw new Error( + `edit at index ${i}: ${EMPTY_NON_FIRST_EDIT_MESSAGE}`, + ); + } + } + } + } + + return isCreating; +} diff --git a/gui/src/util/clientTools/multiEditImpl.test.ts b/gui/src/util/clientTools/multiEditImpl.test.ts new file mode 100644 index 00000000000..7c262cb8246 --- /dev/null +++ b/gui/src/util/clientTools/multiEditImpl.test.ts @@ -0,0 +1,359 @@ +import * as ideUtils from "core/util/ideUtils"; +import { beforeEach, describe, expect, it, Mock, vi } from "vitest"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; +import { ClientToolExtras } from "./callClientTool"; +import { FOUND_MULTIPLE_FIND_STRINGS_ERROR } from "./findAndReplaceUtils"; +import { multiEditImpl } from "./multiEditImpl"; + +vi.mock("uuid", () => ({ + v4: vi.fn(() => "test-uuid"), +})); + +vi.mock("core/util/ideUtils", () => ({ + resolveRelativePathInDir: vi.fn(), + inferResolvedUriFromRelativePath: vi.fn(), +})); + +vi.mock("../../redux/thunks/handleApplyStateUpdate", () => ({ + applyForEditTool: vi.fn(), +})); + +describe("multiEditImpl", () => { + let mockExtras: ClientToolExtras; + let mockResolveRelativePathInDir: Mock; + let mockInferResolvedUriFromRelativePath: Mock; + let mockApplyForEditTool: Mock; + + beforeEach(() => { + vi.clearAllMocks(); + + mockResolveRelativePathInDir = vi.mocked(ideUtils.resolveRelativePathInDir); + mockInferResolvedUriFromRelativePath = vi.mocked( + ideUtils.inferResolvedUriFromRelativePath, + ); + mockApplyForEditTool = vi.mocked(applyForEditTool); + + mockExtras = { + getState: vi.fn(() => ({ + config: { config: { allowAnonymousTelemetry: false } }, + })) as any, + dispatch: vi.fn() as any, + ideMessenger: { + ide: { + readFile: vi.fn(), + getWorkspaceDirs: vi.fn().mockResolvedValue(["dir1"]), + }, + request: vi.fn(), + } as any, + }; + }); + + describe("argument validation", () => { + it("should throw if filepath is missing", async () => { + await expect( + multiEditImpl({ edits: [] }, "id", mockExtras), + ).rejects.toThrow("filepath is required"); + }); + + it("should throw if edits array is empty", async () => { + await expect( + multiEditImpl({ filepath: "test.txt", edits: [] }, "id", mockExtras), + ).rejects.toThrow( + "edits array is required and must contain at least one edit", + ); + }); + + it("should throw if edit has invalid old_string", async () => { + await expect( + multiEditImpl( + { + filepath: "test.txt", + edits: [{ old_string: null, new_string: "test" }], + }, + "id", + mockExtras, + ), + ).rejects.toThrow("edit at index 0: old_string is required"); + }); + + it("should throw if edit has undefined new_string", async () => { + await expect( + multiEditImpl( + { + filepath: "test.txt", + edits: [{ old_string: "test", new_string: undefined }], + }, + "id", + mockExtras, + ), + ).rejects.toThrow("edit at index 0: new_string is required"); + }); + + it("should throw if old_string equals new_string", async () => { + await expect( + multiEditImpl( + { + filepath: "test.txt", + edits: [{ old_string: "same", new_string: "same" }], + }, + "id", + mockExtras, + ), + ).rejects.toThrow( + "edit at index 0: old_string and new_string must be different", + ); + }); + }); + + describe("sequential edits", () => { + beforeEach(() => { + mockResolveRelativePathInDir.mockResolvedValue( + "file:///dir/test/file.txt", + ); + }); + + it("should apply single edit", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world"); + + await multiEditImpl( + { + filepath: "file.txt", + edits: [{ old_string: "Hello", new_string: "Hi" }], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "Hi world", + filepath: "file:///dir/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should apply multiple edits sequentially", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world\nGoodbye world"); + + await multiEditImpl( + { + filepath: "file.txt", + edits: [ + { old_string: "Hello", new_string: "Hi" }, + { old_string: "world", new_string: "universe", replace_all: true }, + ], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "Hi universe\nGoodbye universe", + filepath: "file:///dir/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should handle edits that depend on previous edits", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("const x = 1;"); + + await multiEditImpl( + { + filepath: "file.txt", + edits: [ + { old_string: "const x", new_string: "let x" }, + { old_string: "let x = 1;", new_string: "let x = 2;" }, + ], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "let x = 2;", + filepath: "file:///dir/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should throw if string not found in edit sequence", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world"); + + await expect( + multiEditImpl( + { + filepath: "file.txt", + edits: [ + { old_string: "Hello", new_string: "Hi" }, + { old_string: "not found", new_string: "test" }, + ], + }, + "id", + mockExtras, + ), + ).rejects.toThrow( + 'edit at index 1: string not found in file: "not found"', + ); + }); + + it("should throw if multiple occurrences without replace_all", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("test test test"); + + await expect( + multiEditImpl( + { + filepath: "file.txt", + edits: [{ old_string: "test", new_string: "replaced" }], + }, + "id", + mockExtras, + ), + ).rejects.toThrow( + `edit at index 0: String "test" appears 3 times in the file. ${FOUND_MULTIPLE_FIND_STRINGS_ERROR}`, + ); + }); + }); + + describe("file creation", () => { + it("should create new file with empty old_string", async () => { + mockResolveRelativePathInDir.mockResolvedValue(null); + mockInferResolvedUriFromRelativePath.mockResolvedValue( + "file:///infered/new.txt", + ); + + await multiEditImpl( + { + filepath: "new.txt", + edits: [{ old_string: "", new_string: "New content\nLine 2" }], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "New content\nLine 2", + filepath: "file:///infered/new.txt", + isSearchAndReplace: true, + }); + }); + }); + + describe("replace_all functionality", () => { + beforeEach(() => { + mockResolveRelativePathInDir.mockResolvedValue( + "file:///dir/test/file.txt", + ); + }); + + it("should replace all occurrences when specified", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("foo bar foo baz foo"); + + await multiEditImpl( + { + filepath: "file.txt", + edits: [{ old_string: "foo", new_string: "qux", replace_all: true }], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "qux bar qux baz qux", + filepath: "file:///dir/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should handle mixed replace_all settings", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("x y x z x"); + + await multiEditImpl( + { + filepath: "file.txt", + edits: [ + { old_string: "x", new_string: "a", replace_all: true }, + { old_string: "y", new_string: "b" }, + ], + }, + "id", + mockExtras, + ); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "id", + text: "a b a z a", + filepath: "file:///dir/test/file.txt", + isSearchAndReplace: true, + }); + }); + }); + + describe("error handling", () => { + it("should wrap readFile errors", async () => { + mockResolveRelativePathInDir.mockResolvedValue( + "file:///dir/test/file.txt", + ); + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockRejectedValue(new Error("Read failed")); + + await expect( + multiEditImpl( + { + filepath: "file.txt", + edits: [{ old_string: "test", new_string: "new" }], + }, + "id", + mockExtras, + ), + ).rejects.toThrow("Read failed"); + }); + }); + + describe("return value", () => { + it("should return correct structure", async () => { + mockResolveRelativePathInDir.mockResolvedValue( + "file:///dir/test/file.txt", + ); + mockExtras.ideMessenger.ide.readFile = vi.fn().mockResolvedValue("test"); + + const result = await multiEditImpl( + { + filepath: "file.txt", + edits: [{ old_string: "test", new_string: "new" }], + }, + "id", + mockExtras, + ); + + expect(result).toEqual({ + respondImmediately: false, + output: undefined, + }); + }); + }); +}); diff --git a/gui/src/util/clientTools/multiEditImpl.ts b/gui/src/util/clientTools/multiEditImpl.ts new file mode 100644 index 00000000000..8b12109e5ad --- /dev/null +++ b/gui/src/util/clientTools/multiEditImpl.ts @@ -0,0 +1,99 @@ +import { + inferResolvedUriFromRelativePath, + resolveRelativePathInDir, +} from "core/util/ideUtils"; +import { v4 as uuid } from "uuid"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; +import { ClientToolImpl } from "./callClientTool"; +import { + performFindAndReplace, + validateCreatingForMultiEdit, + validateSingleEdit, +} from "./findAndReplaceUtils"; + +export const multiEditImpl: ClientToolImpl = async ( + args, + toolCallId, + extras, +) => { + const { filepath, edits, editingFileContents } = args; + + const streamId = uuid(); + + // Validate arguments + if (!filepath) { + throw new Error("filepath is required"); + } + if (!edits || !Array.isArray(edits) || edits.length === 0) { + throw new Error( + "edits array is required and must contain at least one edit", + ); + } + + // Validate each edit operation + for (let i = 0; i < edits.length; i++) { + const edit = edits[i]; + validateSingleEdit(edit.old_string, edit.new_string, i); + } + + // Check if this is creating a new file (first edit has empty old_string) + const isCreatingNewFile = validateCreatingForMultiEdit(edits); + const resolvedUri = await resolveRelativePathInDir( + filepath, + extras.ideMessenger.ide, + ); + + let newContent: string; + let fileUri: string; + if (isCreatingNewFile) { + if (resolvedUri) { + throw new Error( + `file ${filepath} already exists, cannot create new file`, + ); + } + newContent = edits[0].new_string; + const dirs = await extras.ideMessenger.ide.getWorkspaceDirs(); + fileUri = await inferResolvedUriFromRelativePath( + filepath, + extras.ideMessenger.ide, + dirs, + ); + } else { + if (!resolvedUri) { + throw new Error( + `file ${filepath} does not exist. If you are trying to edit it, correct the filepath. If you are trying to create it, you must pass old_string=""`, + ); + } + newContent = + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedUri)); + fileUri = resolvedUri; + for (let i = 0; i < edits.length; i++) { + const { old_string, new_string, replace_all } = edits[i]; + newContent = performFindAndReplace( + newContent, + old_string, + new_string, + replace_all, + i, + ); + } + } + + // Apply the changes to the file + void extras.dispatch( + applyForEditTool({ + streamId, + toolCallId, + text: newContent, + filepath: fileUri, + isSearchAndReplace: true, + }), + ); + + // Return success - applyToFile will handle the completion state + return { + respondImmediately: false, // Let apply state handle completion + output: undefined, + }; +}; diff --git a/gui/src/util/clientTools/searchReplaceImpl.test.ts b/gui/src/util/clientTools/searchReplaceImpl.test.ts index 8d8e2ed07fc..69423a13033 100644 --- a/gui/src/util/clientTools/searchReplaceImpl.test.ts +++ b/gui/src/util/clientTools/searchReplaceImpl.test.ts @@ -2,6 +2,7 @@ import { findSearchMatch } from "core/edit/searchAndReplace/findSearchMatch"; import { parseAllSearchReplaceBlocks } from "core/edit/searchAndReplace/parseSearchReplaceBlock"; import { resolveRelativePathInDir } from "core/util/ideUtils"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; import { ClientToolExtras } from "./callClientTool"; import { searchReplaceToolImpl } from "./searchReplaceImpl"; @@ -12,10 +13,14 @@ vi.mock("core/util/ideUtils"); vi.mock("uuid", () => ({ v4: vi.fn(() => "test-stream-id"), })); +vi.mock("../../redux/thunks/handleApplyStateUpdate", () => ({ + applyForEditTool: vi.fn(), +})); const mockFindSearchMatch = vi.mocked(findSearchMatch); const mockParseAllSearchReplaceBlocks = vi.mocked(parseAllSearchReplaceBlocks); const mockResolveRelativePathInDir = vi.mocked(resolveRelativePathInDir); +const mockApplyForEditTool = vi.mocked(applyForEditTool); describe("searchReplaceToolImpl", () => { let mockExtras: ClientToolExtras; @@ -144,8 +149,6 @@ describe("searchReplaceToolImpl", () => { endIndex, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - const result = await searchReplaceToolImpl( { filepath: "test.js", diffs: ["mock diff content"] }, "tool-call-id", @@ -159,7 +162,7 @@ describe("searchReplaceToolImpl", () => { }); // Verify applyToFile was called with correct parameters - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.js", @@ -202,8 +205,6 @@ describe("searchReplaceToolImpl", () => { endIndex, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - const result = await searchReplaceToolImpl( { filepath: "test.js", diffs: ["mock diff content"] }, "tool-call-id", @@ -217,7 +218,7 @@ describe("searchReplaceToolImpl", () => { }); // Verify applyToFile was called with correct parameters - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.js", @@ -283,8 +284,6 @@ const c = 3;`; strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - const result = await searchReplaceToolImpl( { filepath: "test.js", diffs: ["mock diff content"] }, "tool-call-id", @@ -310,7 +309,7 @@ const c = 3;`; "const b = 2;", ); // Verify final applyToFile call - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.js", @@ -379,8 +378,6 @@ const c = 3;`; strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - const result = await searchReplaceToolImpl( { filepath: "test.js", diffs: ["first diff", "second diff"] }, "tool-call-id", @@ -405,7 +402,7 @@ const c = 3;`; ); // Verify final applyToFile call - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.js", @@ -440,7 +437,6 @@ keep this too`; endIndex: 26, // End of "remove this line" strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); const result = await searchReplaceToolImpl( { filepath: "test.txt", diffs: ["mock diff content"] }, @@ -453,7 +449,7 @@ keep this too`; output: undefined, }); - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.txt", @@ -480,15 +476,13 @@ keep this too`; endIndex: 11, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - await searchReplaceToolImpl( { filepath: "test.txt", diffs: ["mock diff content"] }, "tool-call-id", mockExtras, ); - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.txt", @@ -602,15 +596,13 @@ keep this too`; endIndex: 0, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); - await searchReplaceToolImpl( { filepath: "test.txt", diffs: ["mock diff content"] }, "tool-call-id", mockExtras, ); - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.txt", @@ -637,7 +629,6 @@ keep this too`; endIndex: originalContent.length, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); await searchReplaceToolImpl( { filepath: "test.txt", diffs: ["mock diff content"] }, @@ -645,7 +636,7 @@ keep this too`; mockExtras, ); - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: expectedFinalContent, streamId: "test-stream-id", filepath: "/resolved/path/test.txt", @@ -673,7 +664,6 @@ keep this too`; endIndex: 4, strategyName: "exactMatch", }); - mockIdeMessenger.request.mockResolvedValue({ status: "success" }); await searchReplaceToolImpl( { filepath: "relative/test.txt", diffs: ["mock diff content"] }, @@ -693,7 +683,7 @@ keep this too`; "/resolved/path/test.txt", ); expect(mockFindSearchMatch).toHaveBeenCalledWith(originalContent, "test"); - expect(mockIdeMessenger.request).toHaveBeenCalledWith("applyToFile", { + expect(mockApplyForEditTool).toHaveBeenCalledWith({ text: "updated content", streamId: "test-stream-id", filepath: "/resolved/path/test.txt", diff --git a/gui/src/util/clientTools/searchReplaceImpl.ts b/gui/src/util/clientTools/searchReplaceImpl.ts index b734b61ca02..5b750b26f57 100644 --- a/gui/src/util/clientTools/searchReplaceImpl.ts +++ b/gui/src/util/clientTools/searchReplaceImpl.ts @@ -3,8 +3,7 @@ import { parseAllSearchReplaceBlocks } from "core/edit/searchAndReplace/parseSea import { resolveRelativePathInDir } from "core/util/ideUtils"; import posthog from "posthog-js"; import { v4 as uuid } from "uuid"; -import { updateApplyState } from "../../redux/slices/sessionSlice"; -import { handleEditToolApplyError } from "../../redux/thunks/handleApplyStateUpdate"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; import { ClientToolImpl } from "./callClientTool"; export const searchReplaceToolImpl: ClientToolImpl = async ( @@ -12,7 +11,7 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( toolCallId, extras, ) => { - const { filepath, diffs } = args; + const { filepath, diffs, editingFileContents } = args; const state = extras.getState(); const allowAnonymousTelemetry = state.config.config.allowAnonymousTelemetry; @@ -47,7 +46,8 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( try { // Read the current file content const originalContent = - await extras.ideMessenger.ide.readFile(resolvedFilepath); + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedFilepath)); let currentContent = originalContent; // Apply all replacements sequentially to build the final content @@ -84,30 +84,15 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( // This works becaues of our logic in `applyCodeBlock` that determines // that the full file rewrite here can be applied instantly, so the diff // lines are generated with meyers diff and streamed instantaneously - extras.dispatch( - updateApplyState({ - streamId, - toolCallId, - status: "not-started", - }), - ); - void extras.ideMessenger - .request("applyToFile", { + void extras.dispatch( + applyForEditTool({ streamId, toolCallId, text: currentContent, filepath: resolvedFilepath, isSearchAndReplace: true, - }) - .then((res) => { - if (res.status === "error") { - void extras.dispatch( - handleEditToolApplyError({ - toolCallId, - }), - ); - } - }); + }), + ); // Return success - applyToFile will handle the completion state return { diff --git a/gui/src/util/clientTools/singleFindAndReplaceImpl.test.ts b/gui/src/util/clientTools/singleFindAndReplaceImpl.test.ts new file mode 100644 index 00000000000..e6e8201b9f2 --- /dev/null +++ b/gui/src/util/clientTools/singleFindAndReplaceImpl.test.ts @@ -0,0 +1,334 @@ +import * as ideUtils from "core/util/ideUtils"; +import { beforeEach, describe, expect, it, Mock, vi } from "vitest"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; +import { ClientToolExtras } from "./callClientTool"; +import { singleFindAndReplaceImpl } from "./singleFindAndReplaceImpl"; +vi.mock("uuid", () => ({ + v4: vi.fn(() => "test-uuid"), +})); + +vi.mock("core/util/ideUtils", () => ({ + resolveRelativePathInDir: vi.fn(), +})); + +vi.mock("../../redux/thunks/handleApplyStateUpdate", () => ({ + applyForEditTool: vi.fn(), +})); + +describe("singleFindAndReplaceImpl", () => { + let mockExtras: ClientToolExtras; + let mockResolveRelativePathInDir: Mock; + let mockApplyForEditTool: Mock; + + beforeEach(() => { + vi.clearAllMocks(); + + mockResolveRelativePathInDir = vi.mocked(ideUtils.resolveRelativePathInDir); + mockApplyForEditTool = vi.mocked(applyForEditTool); + + mockExtras = { + getState: vi.fn(() => ({ + config: { + config: { + allowAnonymousTelemetry: false, + }, + }, + })) as any, + dispatch: vi.fn() as any, + ideMessenger: { + ide: { + readFile: vi.fn(), + }, + request: vi.fn(), + } as any, + }; + }); + + describe("argument validation", () => { + it("should throw error if filepath is missing", async () => { + const args = { + old_string: "test", + new_string: "replacement", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("filepath is required"); + }); + + it("should throw error if old_string is missing", async () => { + const args = { + filepath: "test.txt", + new_string: "replacement", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("old_string is required"); + }); + + it("should throw error if new_string is missing", async () => { + const args = { + filepath: "test.txt", + old_string: "test", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("new_string is required"); + }); + + it("should throw error if old_string and new_string are the same", async () => { + const args = { + filepath: "test.txt", + old_string: "same", + new_string: "same", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("old_string and new_string must be different"); + }); + }); + + describe("file resolution", () => { + it("should throw error if file does not exist", async () => { + mockResolveRelativePathInDir.mockResolvedValue(null); + + const args = { + filepath: "nonexistent.txt", + old_string: "test", + new_string: "replacement", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("File nonexistent.txt does not exist"); + }); + + it("should resolve relative file paths", async () => { + mockResolveRelativePathInDir.mockResolvedValue("/absolute/path/test.txt"); + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("test content"); + + const args = { + filepath: "test.txt", + old_string: "test", + new_string: "replacement", + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + expect(mockResolveRelativePathInDir).toHaveBeenCalledWith( + "test.txt", + mockExtras.ideMessenger.ide, + ); + expect(mockExtras.ideMessenger.ide.readFile).toHaveBeenCalledWith( + "/absolute/path/test.txt", + ); + }); + }); + + describe("string replacement", () => { + beforeEach(() => { + mockResolveRelativePathInDir.mockResolvedValue("/test/file.txt"); + }); + + it("should throw error if old_string is not found in file", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("different content"); + + const args = { + filepath: "file.txt", + old_string: "not found", + new_string: "replacement", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow(`string not found in file: "not found"`); + }); + + it("should replace single occurrence", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world\nThis is a test file\nGoodbye world"); + + const args = { + filepath: "file.txt", + old_string: "Hello world", + new_string: "Hi there", + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + // Check that the dispatch was called with the applyForEditTool thunk + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "tool-call-id", + text: "Hi there\nThis is a test file\nGoodbye world", + filepath: "/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should throw error if old_string appears multiple times and replace_all is false", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world\nThis is a test file\nGoodbye world"); + + const args = { + filepath: "file.txt", + old_string: "world", + new_string: "universe", + replace_all: false, + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow( + 'String "world" appears 2 times in the file. Either provide a more specific string with surrounding context to make it unique, or use replace_all=true to replace all occurrences.', + ); + }); + + it("should replace all occurrences when replace_all is true", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world\nThis is a test file\nGoodbye world"); + + const args = { + filepath: "file.txt", + old_string: "world", + new_string: "universe", + replace_all: true, + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "tool-call-id", + text: "Hello universe\nThis is a test file\nGoodbye universe", + filepath: "/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should handle empty new_string (deletion)", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("Hello world\nThis is a test file"); + + const args = { + filepath: "file.txt", + old_string: "Hello ", + new_string: "", + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "tool-call-id", + text: "world\nThis is a test file", + filepath: "/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should handle special characters in strings", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue( + 'const regex = /[a-z]+/g;\nconst text = "Hello $world"', + ); + + const args = { + filepath: "file.txt", + old_string: '"Hello $world"', + new_string: '"Hi $universe"', + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "tool-call-id", + text: 'const regex = /[a-z]+/g;\nconst text = "Hi $universe"', + filepath: "/test/file.txt", + isSearchAndReplace: true, + }); + }); + + it("should preserve whitespace and indentation", async () => { + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue( + "function test() {\n const value = 'old';\n return value;\n}", + ); + + const args = { + filepath: "file.txt", + old_string: " const value = 'old';", + new_string: " const value = 'new';", + }; + + await singleFindAndReplaceImpl(args, "tool-call-id", mockExtras); + + expect(mockApplyForEditTool).toHaveBeenCalledWith({ + streamId: "test-uuid", + toolCallId: "tool-call-id", + text: "function test() {\n const value = 'new';\n return value;\n}", + filepath: "/test/file.txt", + isSearchAndReplace: true, + }); + }); + }); + + describe("return value", () => { + it("should return correct response structure", async () => { + mockResolveRelativePathInDir.mockResolvedValue("/test/file.txt"); + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockResolvedValue("test content"); + + const args = { + filepath: "file.txt", + old_string: "test", + new_string: "replacement", + }; + + const result = await singleFindAndReplaceImpl( + args, + "tool-call-id", + mockExtras, + ); + + expect(result).toEqual({ + respondImmediately: false, + output: undefined, + }); + }); + }); + + describe("error handling", () => { + it("should wrap and rethrow errors from readFile", async () => { + mockResolveRelativePathInDir.mockResolvedValue("/test/file.txt"); + mockExtras.ideMessenger.ide.readFile = vi + .fn() + .mockRejectedValue(new Error("Permission denied")); + + const args = { + filepath: "file.txt", + old_string: "test", + new_string: "replacement", + }; + + await expect( + singleFindAndReplaceImpl(args, "tool-call-id", mockExtras), + ).rejects.toThrow("Permission denied"); + }); + }); +}); diff --git a/gui/src/util/clientTools/singleFindAndReplaceImpl.ts b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts new file mode 100644 index 00000000000..92c3da17838 --- /dev/null +++ b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts @@ -0,0 +1,69 @@ +import { resolveRelativePathInDir } from "core/util/ideUtils"; +import { v4 as uuid } from "uuid"; +import { applyForEditTool } from "../../redux/thunks/handleApplyStateUpdate"; +import { ClientToolImpl } from "./callClientTool"; +import { + performFindAndReplace, + validateSingleEdit, +} from "./findAndReplaceUtils"; + +export const singleFindAndReplaceImpl: ClientToolImpl = async ( + args, + toolCallId, + extras, +) => { + const { + filepath, + old_string, + new_string, + replace_all = false, + editingFileContents, + } = args; + + const streamId = uuid(); + + // Validate arguments + if (!filepath) { + throw new Error("filepath is required"); + } + validateSingleEdit(old_string, new_string); + + // Resolve the file path + const resolvedFilepath = await resolveRelativePathInDir( + filepath, + extras.ideMessenger.ide, + ); + if (!resolvedFilepath) { + throw new Error(`File ${filepath} does not exist`); + } + + // Read the current file content + const originalContent = + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedFilepath)); + + // Perform the find and replace operation + const newContent = performFindAndReplace( + originalContent, + old_string, + new_string, + replace_all, + ); + + // Apply the changes to the file + void extras.dispatch( + applyForEditTool({ + streamId, + toolCallId, + text: newContent, + filepath: resolvedFilepath, + isSearchAndReplace: true, + }), + ); + + // Return success - applyToFile will handle the completion state + return { + respondImmediately: false, // Let apply state handle completion + output: undefined, + }; +};