diff --git a/packages/coding-agent/src/core/tools/edit-diff.ts b/packages/coding-agent/src/core/tools/edit-diff.ts index 4077e747..24b0828f 100644 --- a/packages/coding-agent/src/core/tools/edit-diff.ts +++ b/packages/coding-agent/src/core/tools/edit-diff.ts @@ -70,6 +70,23 @@ export interface FuzzyMatchResult { contentForReplacement: string; } +export interface ReplaceEdit { + oldText: string; + newText: string; +} + +interface MatchedEdit { + editIndex: number; + matchIndex: number; + matchLength: number; + newText: string; +} + +export interface AppliedEditsResult { + baseContent: string; + newContent: string; +} + /** * Find oldText in content, trying exact match first, then fuzzy match. * When fuzzy matching is used, the returned contentForReplacement is the @@ -121,6 +138,127 @@ export function stripBom(content: string): { bom: string; text: string } { return content.startsWith("\uFEFF") ? { bom: "\uFEFF", text: content.slice(1) } : { bom: "", text: content }; } +function countOccurrences(content: string, oldText: string): number { + const fuzzyContent = normalizeForFuzzyMatch(content); + const fuzzyOldText = normalizeForFuzzyMatch(oldText); + return fuzzyContent.split(fuzzyOldText).length - 1; +} + +function getNotFoundError(path: string, editIndex: number, totalEdits: number): Error { + if (totalEdits === 1) { + return new Error( + `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`, + ); + } + return new Error( + `Could not find edits[${editIndex}] in ${path}. The oldText must match exactly including all whitespace and newlines.`, + ); +} + +function getDuplicateError(path: string, editIndex: number, totalEdits: number, occurrences: number): Error { + if (totalEdits === 1) { + return new Error( + `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`, + ); + } + return new Error( + `Found ${occurrences} occurrences of edits[${editIndex}] in ${path}. Each oldText must be unique. Please provide more context to make it unique.`, + ); +} + +function getEmptyOldTextError(path: string, editIndex: number, totalEdits: number): Error { + if (totalEdits === 1) { + return new Error(`oldText must not be empty in ${path}.`); + } + return new Error(`edits[${editIndex}].oldText must not be empty in ${path}.`); +} + +function getNoChangeError(path: string, totalEdits: number): Error { + if (totalEdits === 1) { + return new Error( + `No changes made to ${path}. The replacement produced identical content. This might indicate an issue with special characters or the text not existing as expected.`, + ); + } + return new Error(`No changes made to ${path}. The replacements produced identical content.`); +} + +/** + * Apply one or more exact-text replacements to LF-normalized content. + * + * All edits are matched against the same original content. Replacements are + * then applied in reverse order so offsets remain stable. If any edit needs + * fuzzy matching, the operation runs in fuzzy-normalized content space to + * preserve current single-edit behavior. + */ +export function applyEditsToNormalizedContent( + normalizedContent: string, + edits: ReplaceEdit[], + path: string, +): AppliedEditsResult { + const normalizedEdits = edits.map((edit) => ({ + oldText: normalizeToLF(edit.oldText), + newText: normalizeToLF(edit.newText), + })); + + for (let i = 0; i < normalizedEdits.length; i++) { + if (normalizedEdits[i].oldText.length === 0) { + throw getEmptyOldTextError(path, i, normalizedEdits.length); + } + } + + const initialMatches = normalizedEdits.map((edit) => fuzzyFindText(normalizedContent, edit.oldText)); + const baseContent = initialMatches.some((match) => match.usedFuzzyMatch) + ? normalizeForFuzzyMatch(normalizedContent) + : normalizedContent; + + const matchedEdits: MatchedEdit[] = []; + for (let i = 0; i < normalizedEdits.length; i++) { + const edit = normalizedEdits[i]; + const matchResult = fuzzyFindText(baseContent, edit.oldText); + if (!matchResult.found) { + throw getNotFoundError(path, i, normalizedEdits.length); + } + + const occurrences = countOccurrences(baseContent, edit.oldText); + if (occurrences > 1) { + throw getDuplicateError(path, i, normalizedEdits.length, occurrences); + } + + matchedEdits.push({ + editIndex: i, + matchIndex: matchResult.index, + matchLength: matchResult.matchLength, + newText: edit.newText, + }); + } + + matchedEdits.sort((a, b) => a.matchIndex - b.matchIndex); + for (let i = 1; i < matchedEdits.length; i++) { + const previous = matchedEdits[i - 1]; + const current = matchedEdits[i]; + if (previous.matchIndex + previous.matchLength > current.matchIndex) { + throw new Error( + `edits[${previous.editIndex}] and edits[${current.editIndex}] overlap in ${path}. Merge them into one edit or target disjoint regions.`, + ); + } + } + + let newContent = baseContent; + for (let i = matchedEdits.length - 1; i >= 0; i--) { + const edit = matchedEdits[i]; + newContent = + newContent.substring(0, edit.matchIndex) + + edit.newText + + newContent.substring(edit.matchIndex + edit.matchLength); + } + + if (baseContent === newContent) { + throw getNoChangeError(path, normalizedEdits.length); + } + + return { baseContent, newContent }; +} + /** * Generate a unified diff string with line numbers and context. * Returns both the diff string and the first changed line number (in the new file). @@ -237,13 +375,12 @@ export interface EditDiffError { } /** - * Compute the diff for an edit operation without applying it. + * Compute the diff for one or more edit operations without applying them. * Used for preview rendering in the TUI before the tool executes. */ -export async function computeEditDiff( +export async function computeEditsDiff( path: string, - oldText: string, - newText: string, + edits: ReplaceEdit[], cwd: string, ): Promise { const absolutePath = resolveToCwd(path, cwd); @@ -261,45 +398,8 @@ export async function computeEditDiff( // Strip BOM before matching (LLM won't include invisible BOM in oldText) const { text: content } = stripBom(rawContent); - const normalizedContent = normalizeToLF(content); - const normalizedOldText = normalizeToLF(oldText); - const normalizedNewText = normalizeToLF(newText); - - // Find the old text using fuzzy matching (tries exact match first, then fuzzy) - const matchResult = fuzzyFindText(normalizedContent, normalizedOldText); - - if (!matchResult.found) { - return { - error: `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`, - }; - } - - // Count occurrences using fuzzy-normalized content for consistency - const fuzzyContent = normalizeForFuzzyMatch(normalizedContent); - const fuzzyOldText = normalizeForFuzzyMatch(normalizedOldText); - const occurrences = fuzzyContent.split(fuzzyOldText).length - 1; - - if (occurrences > 1) { - return { - error: `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`, - }; - } - - // Compute the new content using the matched position - // When fuzzy matching was used, contentForReplacement is the normalized version - const baseContent = matchResult.contentForReplacement; - const newContent = - baseContent.substring(0, matchResult.index) + - normalizedNewText + - baseContent.substring(matchResult.index + matchResult.matchLength); - - // Check if it would actually change anything - if (baseContent === newContent) { - return { - error: `No changes would be made to ${path}. The replacement produces identical content.`, - }; - } + const { baseContent, newContent } = applyEditsToNormalizedContent(normalizedContent, edits, path); // Generate the diff return generateDiffString(baseContent, newContent); @@ -307,3 +407,16 @@ export async function computeEditDiff( return { error: err instanceof Error ? err.message : String(err) }; } } + +/** + * Compute the diff for a single edit operation without applying it. + * Kept as a convenience wrapper for single-edit callers. + */ +export async function computeEditDiff( + path: string, + oldText: string, + newText: string, + cwd: string, +): Promise { + return computeEditsDiff(path, [{ oldText, newText }], cwd); +} diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 0ee47a08..cb493ea6 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -6,14 +6,14 @@ import { access as fsAccess, readFile as fsReadFile, writeFile as fsWriteFile } import { renderDiff } from "../../modes/interactive/components/diff.js"; import type { ToolDefinition } from "../extensions/types.js"; import { - computeEditDiff, + applyEditsToNormalizedContent, + computeEditsDiff, detectLineEnding, type EditDiffError, type EditDiffResult, - fuzzyFindText, generateDiffString, - normalizeForFuzzyMatch, normalizeToLF, + type ReplaceEdit, restoreLineEndings, stripBom, } from "./edit-diff.js"; @@ -27,14 +27,60 @@ type EditRenderState = { preview?: EditDiffResult | EditDiffError; }; -const editSchema = Type.Object({ - path: Type.String({ description: "Path to the file to edit (relative or absolute)" }), - oldText: Type.String({ description: "Exact text to find and replace (must match exactly)" }), - newText: Type.String({ description: "New text to replace the old text with" }), -}); +const replaceEditSchema = Type.Object( + { + oldText: Type.String({ + description: + "Exact text for one targeted replacement. It must be unique in the original file and must not overlap with any other edits[].oldText in the same call.", + }), + newText: Type.String({ description: "Replacement text for this targeted edit." }), + }, + { additionalProperties: false }, +); + +const editSchema = Type.Object( + { + path: Type.String({ description: "Path to the file to edit (relative or absolute)" }), + oldText: Type.Optional( + Type.String({ + description: + "Exact text to replace for one contiguous change. Include only enough surrounding context to make the match unique. Do not use this to cover multiple distant changes in one large block.", + }), + ), + newText: Type.Optional( + Type.String({ + description: "Replacement text for oldText in single-replacement mode.", + }), + ), + edits: Type.Optional( + Type.Array(replaceEditSchema, { + description: + "Use this when changing multiple separate, disjoint regions in the same file. Each edit is matched against the original file, not incrementally. Do not include overlapping or nested edits. If two changes touch the same block or nearby lines, merge them into one edit instead.", + }), + ), + }, + { additionalProperties: false }, +); export type EditToolInput = Static; +interface ReplaceModeInput { + path: string; + oldText: string; + newText: string; +} + +interface MultiReplaceModeInput { + path: string; + edits: ReplaceEdit[]; +} + +interface NormalizedEditInput { + path: string; + edits: ReplaceEdit[]; + mode: "single" | "multi"; +} + export interface EditToolDetails { /** Unified diff of the changes made */ diff: string; @@ -66,8 +112,73 @@ export interface EditToolOptions { operations?: EditOperations; } +function getMultiReplaceModeInput(input: EditToolInput): MultiReplaceModeInput | null { + if (input.edits === undefined) return null; + if (input.oldText !== undefined || input.newText !== undefined) { + throw new Error("Edit tool input is invalid. Use either edits or single replacement mode, not both."); + } + if (input.edits.length === 0) { + throw new Error("Edit tool input is invalid. edits must contain at least one replacement."); + } + return { path: input.path, edits: input.edits }; +} + +function getReplaceModeInput(input: EditToolInput): ReplaceModeInput | null { + const { oldText, newText } = input; + if (oldText === undefined && newText === undefined) return null; + if (input.edits !== undefined) { + throw new Error("Edit tool input is invalid. Use either single replacement mode or edits mode, not both."); + } + if (oldText === undefined || newText === undefined) { + throw new Error("Edit tool input is invalid. Single replacement mode requires both oldText and newText."); + } + return { path: input.path, oldText, newText }; +} + +function normalizeEditInput(input: EditToolInput): NormalizedEditInput { + const multiReplaceModeInput = getMultiReplaceModeInput(input); + if (multiReplaceModeInput) { + return { path: multiReplaceModeInput.path, edits: multiReplaceModeInput.edits, mode: "multi" }; + } + + const replaceModeInput = getReplaceModeInput(input); + if (replaceModeInput) { + return { + path: replaceModeInput.path, + edits: [{ oldText: replaceModeInput.oldText, newText: replaceModeInput.newText }], + mode: "single", + }; + } + + throw new Error("Edit tool input is invalid. Provide either oldText and newText, or edits."); +} + +function getRenderablePreviewInput( + args: { path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, +): { path: string; edits: ReplaceEdit[] } | null { + if (!args || typeof args.path !== "string") { + return null; + } + + if ( + args.oldText === undefined && + args.newText === undefined && + Array.isArray(args.edits) && + args.edits.length > 0 && + args.edits.every((edit) => typeof edit?.oldText === "string" && typeof edit?.newText === "string") + ) { + return { path: args.path, edits: args.edits }; + } + + if (typeof args.oldText === "string" && typeof args.newText === "string" && args.edits === undefined) { + return { path: args.path, edits: [{ oldText: args.oldText, newText: args.newText }] }; + } + + return null; +} + function formatEditCall( - args: { path?: string; file_path?: string; oldText?: string; newText?: string } | undefined, + args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, state: EditRenderState, theme: typeof import("../../modes/interactive/theme/theme.js").theme, ): string { @@ -89,7 +200,7 @@ function formatEditCall( } function formatEditResult( - args: { path?: string; file_path?: string; oldText?: string; newText?: string } | undefined, + args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, state: EditRenderState, result: { content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>; @@ -124,17 +235,18 @@ export function createEditToolDefinition( name: "edit", label: "edit", description: - "Edit a file by replacing exact text. The oldText must match exactly (including whitespace). Use this for precise, surgical edits.", - promptSnippet: "Make surgical edits to files (find exact text and replace)", - promptGuidelines: ["Use edit for precise changes (old text must match exactly)."], + "Edit a single file using exact text replacement. Use oldText/newText only for one contiguous replacement. Use edits when changing multiple separate, disjoint regions in the same file. In edits mode, every edits[].oldText must match a unique, non-overlapping region of the original file. If two changes affect the same block or nearby lines, merge them into one edit instead of emitting overlapping edits. Do not include large unchanged regions just to connect distant changes. Do not provide both modes at once.", + promptSnippet: + "Make precise file edits with exact text replacement, including multiple disjoint edits in one call", + promptGuidelines: [ + "Use edit for precise changes (old text must match exactly)", + "When changing multiple separate locations in one file, use one edit call with edits[] instead of multiple edit calls", + "Each edits[].oldText is matched against the original file, not after earlier edits are applied. Do not emit overlapping or nested edits. Merge nearby changes into one edit.", + "Keep oldText as small as possible while still being unique in the file. Do not pad with large unchanged regions.", + ], parameters: editSchema, - async execute( - _toolCallId, - { path, oldText, newText }: { path: string; oldText: string; newText: string }, - signal?: AbortSignal, - _onUpdate?, - _ctx?, - ) { + async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, _ctx?) { + const { path, edits, mode } = normalizeEditInput(input); const absolutePath = resolveToCwd(path, cwd); return withFileMutationQueue( @@ -163,7 +275,7 @@ export function createEditToolDefinition( } // Perform the edit operation. - (async () => { + void (async () => { try { // Check if file exists. try { @@ -192,70 +304,19 @@ export function createEditToolDefinition( // Strip BOM before matching. The model will not include an invisible BOM in oldText. const { bom, text: content } = stripBom(rawContent); - const originalEnding = detectLineEnding(content); const normalizedContent = normalizeToLF(content); - const normalizedOldText = normalizeToLF(oldText); - const normalizedNewText = normalizeToLF(newText); - - // Find the old text using fuzzy matching. This tries exact match first, then a normalized fallback. - const matchResult = fuzzyFindText(normalizedContent, normalizedOldText); - - if (!matchResult.found) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `Could not find the exact text in ${path}. The old text must match exactly including all whitespace and newlines.`, - ), - ); - return; - } - - // Count occurrences using fuzzy-normalized content for consistency with the matcher. - const fuzzyContent = normalizeForFuzzyMatch(normalizedContent); - const fuzzyOldText = normalizeForFuzzyMatch(normalizedOldText); - const occurrences = fuzzyContent.split(fuzzyOldText).length - 1; - - if (occurrences > 1) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `Found ${occurrences} occurrences of the text in ${path}. The text must be unique. Please provide more context to make it unique.`, - ), - ); - return; - } + const { baseContent, newContent } = applyEditsToNormalizedContent( + normalizedContent, + edits, + path, + ); // Check if aborted before writing. if (aborted) { return; } - // Perform replacement using the matched text position. - // When fuzzy matching was used, contentForReplacement is the normalized version. - const baseContent = matchResult.contentForReplacement; - const newContent = - baseContent.substring(0, matchResult.index) + - normalizedNewText + - baseContent.substring(matchResult.index + matchResult.matchLength); - - // Verify the replacement actually changed something. - if (baseContent === newContent) { - if (signal) { - signal.removeEventListener("abort", onAbort); - } - reject( - new Error( - `No changes made to ${path}. The replacement produced identical content. This might indicate an issue with special characters or the text not existing as expected.`, - ), - ); - return; - } - const finalContent = bom + restoreLineEndings(newContent, originalEnding); await ops.writeFile(absolutePath, finalContent); @@ -274,19 +335,22 @@ export function createEditToolDefinition( content: [ { type: "text", - text: `Successfully replaced text in ${path}.`, + text: + mode === "single" + ? `Successfully replaced text in ${path}.` + : `Successfully replaced ${edits.length} block(s) in ${path}.`, }, ], details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine }, }); - } catch (error: any) { + } catch (error: unknown) { // Clean up abort handler. if (signal) { signal.removeEventListener("abort", onAbort); } if (!aborted) { - reject(error); + reject(error instanceof Error ? error : new Error(String(error))); } } })(); @@ -294,18 +358,21 @@ export function createEditToolDefinition( ); }, renderCall(args, theme, context) { - const isSingleMode = - typeof args?.path === "string" && typeof args?.oldText === "string" && typeof args?.newText === "string"; - if (context.argsComplete && isSingleMode) { - const argsKey = JSON.stringify({ path: args.path, oldText: args.oldText, newText: args.newText }); - if (context.state.argsKey !== argsKey) { - context.state.argsKey = argsKey; - computeEditDiff(args.path!, args.oldText!, args.newText!, context.cwd).then((preview) => { - if (context.state.argsKey === argsKey) { - context.state.preview = preview; - context.invalidate(); - } - }); + if (context.argsComplete) { + const previewInput = getRenderablePreviewInput( + args as { path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] }, + ); + if (previewInput) { + const argsKey = JSON.stringify({ path: previewInput.path, edits: previewInput.edits }); + if (context.state.argsKey !== argsKey) { + context.state.argsKey = argsKey; + computeEditsDiff(previewInput.path, previewInput.edits, context.cwd).then((preview) => { + if (context.state.argsKey === argsKey) { + context.state.preview = preview; + context.invalidate(); + } + }); + } } } const text = (context.lastComponent as Text | undefined) ?? new Text("", 0, 0); diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 3070e7e9..103d93dd 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -258,6 +258,98 @@ describe("Coding Agent Tools", () => { }), ).rejects.toThrow(/Found 3 occurrences/); }); + + it("should replace multiple disjoint regions in one call", async () => { + const testFile = join(testDir, "edit-multi.txt"); + writeFileSync(testFile, "alpha\nbeta\ngamma\ndelta\n"); + + const result = await editTool.execute("test-call-8", { + path: testFile, + edits: [ + { oldText: "alpha\n", newText: "ALPHA\n" }, + { oldText: "gamma\n", newText: "GAMMA\n" }, + ], + }); + + expect(getTextOutput(result)).toContain("Successfully replaced 2 block(s)"); + expect(readFileSync(testFile, "utf-8")).toBe("ALPHA\nbeta\nGAMMA\ndelta\n"); + expect(result.details?.diff).toContain("ALPHA"); + expect(result.details?.diff).toContain("GAMMA"); + }); + + it("should match edits against the original file, not incrementally", async () => { + const testFile = join(testDir, "edit-multi-original.txt"); + writeFileSync(testFile, "foo\nbar\nbaz\n"); + + await editTool.execute("test-call-9", { + path: testFile, + edits: [ + { oldText: "foo\n", newText: "foo bar\n" }, + { oldText: "bar\n", newText: "BAR\n" }, + ], + }); + + expect(readFileSync(testFile, "utf-8")).toBe("foo bar\nBAR\nbaz\n"); + }); + + it("should fail when mixing single-edit mode with edits mode", async () => { + const testFile = join(testDir, "edit-invalid-mixed.txt"); + writeFileSync(testFile, "hello\nworld\n"); + + await expect( + editTool.execute("test-call-10", { + path: testFile, + oldText: "hello\n", + newText: "HELLO\n", + edits: [{ oldText: "world\n", newText: "WORLD\n" }], + }), + ).rejects.toThrow(/Use either/); + }); + + it("should fail when edits is empty", async () => { + const testFile = join(testDir, "edit-empty-edits.txt"); + writeFileSync(testFile, "hello\nworld\n"); + + await expect( + editTool.execute("test-call-11", { + path: testFile, + edits: [], + }), + ).rejects.toThrow(/edits must contain at least one replacement/); + }); + + it("should fail when multi-edit regions overlap", async () => { + const testFile = join(testDir, "edit-overlap.txt"); + writeFileSync(testFile, "one\ntwo\nthree\n"); + + await expect( + editTool.execute("test-call-12", { + path: testFile, + edits: [ + { oldText: "one\ntwo\n", newText: "ONE\nTWO\n" }, + { oldText: "two\nthree\n", newText: "TWO\nTHREE\n" }, + ], + }), + ).rejects.toThrow(/overlap/); + }); + + it("should not partially apply edits when one edit fails", async () => { + const testFile = join(testDir, "edit-no-partial.txt"); + const originalContent = "alpha\nbeta\ngamma\n"; + writeFileSync(testFile, originalContent); + + await expect( + editTool.execute("test-call-13", { + path: testFile, + edits: [ + { oldText: "alpha\n", newText: "ALPHA\n" }, + { oldText: "missing\n", newText: "MISSING\n" }, + ], + }), + ).rejects.toThrow(/Could not find/); + + expect(readFileSync(testFile, "utf-8")).toBe(originalContent); + }); }); describe("bash tool", () => { @@ -603,6 +695,21 @@ describe("edit tool fuzzy matching", () => { }), ).rejects.toThrow(/Found 2 occurrences/); }); + + it("should support fuzzy matching in multi-edit mode", async () => { + const testFile = join(testDir, "fuzzy-multi.txt"); + writeFileSync(testFile, "console.log(\u2018hello\u2019);\nhello\u00A0world\n"); + + await editTool.execute("test-fuzzy-9", { + path: testFile, + edits: [ + { oldText: "console.log('hello');\n", newText: "console.log('world');\n" }, + { oldText: "hello world\n", newText: "hello universe\n" }, + ], + }); + + expect(readFileSync(testFile, "utf-8")).toBe("console.log('world');\nhello universe\n"); + }); }); describe("edit tool CRLF handling", () => { @@ -686,4 +793,20 @@ describe("edit tool CRLF handling", () => { const content = readFileSync(testFile, "utf-8"); expect(content).toBe("\uFEFFfirst\r\nREPLACED\r\nthird\r\n"); }); + + it("should preserve CRLF line endings and BOM in multi-edit mode", async () => { + const testFile = join(testDir, "bom-crlf-multi.txt"); + writeFileSync(testFile, "\uFEFFfirst\r\nsecond\r\nthird\r\nfourth\r\n"); + + await editTool.execute("test-crlf-multi", { + path: testFile, + edits: [ + { oldText: "second\n", newText: "SECOND\n" }, + { oldText: "fourth\n", newText: "FOURTH\n" }, + ], + }); + + const content = readFileSync(testFile, "utf-8"); + expect(content).toBe("\uFEFFfirst\r\nSECOND\r\nthird\r\nFOURTH\r\n"); + }); });