diff --git a/packages/coding-agent/src/core/tools/edit-diff.ts b/packages/coding-agent/src/core/tools/edit-diff.ts index 161d3909..bc47f56e 100644 --- a/packages/coding-agent/src/core/tools/edit-diff.ts +++ b/packages/coding-agent/src/core/tools/edit-diff.ts @@ -70,7 +70,7 @@ export interface FuzzyMatchResult { contentForReplacement: string; } -export interface ReplaceEdit { +export interface Edit { oldText: string; newText: string; } @@ -192,7 +192,7 @@ function getNoChangeError(path: string, totalEdits: number): Error { */ export function applyEditsToNormalizedContent( normalizedContent: string, - edits: ReplaceEdit[], + edits: Edit[], path: string, ): AppliedEditsResult { const normalizedEdits = edits.map((edit) => ({ @@ -403,7 +403,7 @@ export interface EditDiffError { */ export async function computeEditsDiff( path: string, - edits: ReplaceEdit[], + edits: Edit[], cwd: string, ): Promise { const absolutePath = resolveToCwd(path, cwd); diff --git a/packages/coding-agent/src/core/tools/edit.ts b/packages/coding-agent/src/core/tools/edit.ts index 210f50b9..ac37d1e0 100644 --- a/packages/coding-agent/src/core/tools/edit.ts +++ b/packages/coding-agent/src/core/tools/edit.ts @@ -9,11 +9,11 @@ import { applyEditsToNormalizedContent, computeEditsDiff, detectLineEnding, + type Edit, type EditDiffError, type EditDiffResult, generateDiffString, normalizeToLF, - type ReplaceEdit, restoreLineEndings, stripBom, } from "./edit-diff.js"; @@ -41,46 +41,16 @@ const replaceEditSchema = Type.Object( 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.", - }), - ), + edits: Type.Array(replaceEditSchema, { + description: + "One or more targeted replacements. 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; @@ -112,10 +82,11 @@ 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."); +function validateEditInput(input: EditToolInput): { path: string; edits: Edit[] } { + if (!Array.isArray(input.edits)) { + throw new Error( + "Edit tool input is invalid. edits must be an array of replacements in the form { oldText: string, newText: string }.", + ); } if (input.edits.length === 0) { throw new Error("Edit tool input is invalid. edits must contain at least one replacement."); @@ -123,46 +94,20 @@ function getMultiReplaceModeInput(input: EditToolInput): MultiReplaceModeInput | 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 }; -} +type RenderableEditArgs = { + path?: string; + file_path?: string; + edits?: Edit[]; + oldText?: string; + newText?: string; +}; -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 { +function getRenderablePreviewInput(args: RenderableEditArgs | undefined): { path: string; edits: Edit[] } | 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") @@ -170,7 +115,7 @@ function getRenderablePreviewInput( return { path: args.path, edits: args.edits }; } - if (typeof args.oldText === "string" && typeof args.newText === "string" && args.edits === undefined) { + if (typeof args.oldText === "string" && typeof args.newText === "string") { return { path: args.path, edits: [{ oldText: args.oldText, newText: args.newText }] }; } @@ -178,7 +123,7 @@ function getRenderablePreviewInput( } function formatEditCall( - args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, + args: RenderableEditArgs | undefined, state: EditRenderState, theme: typeof import("../../modes/interactive/theme/theme.js").theme, ): string { @@ -200,7 +145,7 @@ function formatEditCall( } function formatEditResult( - args: { path?: string; file_path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] } | undefined, + args: RenderableEditArgs | undefined, state: EditRenderState, result: { content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>; @@ -242,18 +187,18 @@ export function createEditToolDefinition( name: "edit", label: "edit", description: - "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.", + "Edit a single file using exact text replacement. 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.", 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", + "Use edit for precise changes (edits[].oldText must match exactly)", + "When changing multiple separate locations in one file, use one edit call with multiple entries in 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.", + "Keep edits[].oldText as small as possible while still being unique in the file. Do not pad with large unchanged regions.", ], parameters: editSchema, async execute(_toolCallId, input: EditToolInput, signal?: AbortSignal, _onUpdate?, _ctx?) { - const { path, edits, mode } = normalizeEditInput(input); + const { path, edits } = validateEditInput(input); const absolutePath = resolveToCwd(path, cwd); return withFileMutationQueue( @@ -342,10 +287,7 @@ export function createEditToolDefinition( content: [ { type: "text", - text: - mode === "single" - ? `Successfully replaced text in ${path}.` - : `Successfully replaced ${edits.length} block(s) in ${path}.`, + text: `Successfully replaced ${edits.length} block(s) in ${path}.`, }, ], details: { diff: diffResult.diff, firstChangedLine: diffResult.firstChangedLine }, @@ -366,9 +308,7 @@ export function createEditToolDefinition( }, renderCall(args, theme, context) { if (context.argsComplete) { - const previewInput = getRenderablePreviewInput( - args as { path?: string; oldText?: string; newText?: string; edits?: ReplaceEdit[] }, - ); + const previewInput = getRenderablePreviewInput(args as RenderableEditArgs); if (previewInput) { const argsKey = JSON.stringify({ path: previewInput.path, edits: previewInput.edits }); if (context.state.argsKey !== argsKey) { diff --git a/packages/coding-agent/test/file-mutation-queue.test.ts b/packages/coding-agent/test/file-mutation-queue.test.ts index 561f2814..8b839fe2 100644 --- a/packages/coding-agent/test/file-mutation-queue.test.ts +++ b/packages/coding-agent/test/file-mutation-queue.test.ts @@ -108,8 +108,8 @@ describe("built-in edit and write tools", () => { }); await Promise.all([ - editTool.execute("call-1", { path: filePath, oldText: "alpha", newText: "ALPHA" }), - editTool.execute("call-2", { path: filePath, oldText: "beta", newText: "BETA" }), + editTool.execute("call-1", { path: filePath, edits: [{ oldText: "alpha", newText: "ALPHA" }] }), + editTool.execute("call-2", { path: filePath, edits: [{ oldText: "beta", newText: "BETA" }] }), ]); const content = await readFile(filePath, "utf8"); @@ -147,8 +147,7 @@ describe("built-in edit and write tools", () => { const editPromise = editTool.execute("call-1", { path: filePath, - oldText: "original", - newText: "edited", + edits: [{ oldText: "original", newText: "edited" }], }); await delay(5); const writePromise = writeTool.execute("call-2", { diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 020fe094..487a5f9e 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -220,8 +220,7 @@ describe("Coding Agent Tools", () => { const result = await editTool.execute("test-call-5", { path: testFile, - oldText: "world", - newText: "testing", + edits: [{ oldText: "world", newText: "testing" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -239,8 +238,7 @@ describe("Coding Agent Tools", () => { await expect( editTool.execute("test-call-6", { path: testFile, - oldText: "nonexistent", - newText: "testing", + edits: [{ oldText: "nonexistent", newText: "testing" }], }), ).rejects.toThrow(/Could not find the exact text/); }); @@ -253,8 +251,7 @@ describe("Coding Agent Tools", () => { await expect( editTool.execute("test-call-7", { path: testFile, - oldText: "foo", - newText: "bar", + edits: [{ oldText: "foo", newText: "bar" }], }), ).rejects.toThrow(/Found 3 occurrences/); }); @@ -315,20 +312,6 @@ describe("Coding Agent Tools", () => { 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"); @@ -569,8 +552,7 @@ describe("edit tool fuzzy matching", () => { // oldText without trailing whitespace should still match const result = await editTool.execute("test-fuzzy-1", { path: testFile, - oldText: "line one\nline two\n", - newText: "replaced\n", + edits: [{ oldText: "line one\nline two\n", newText: "replaced\n" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -584,8 +566,7 @@ describe("edit tool fuzzy matching", () => { const result = await editTool.execute("test-fuzzy-chinese", { path: testFile, - oldText: "你好,世界\n你好(世界)\n", - newText: "你好,pi\n你好(pi)\n", + edits: [{ oldText: "你好,世界\n你好(世界)\n", newText: "你好,pi\n你好(pi)\n" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -599,8 +580,7 @@ describe("edit tool fuzzy matching", () => { const result = await editTool.execute("test-fuzzy-unicode", { path: testFile, - oldText: "ABC123\ncafé\n", - newText: "XYZ789\ncoffee\n", + edits: [{ oldText: "ABC123\ncafé\n", newText: "XYZ789\ncoffee\n" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -616,8 +596,7 @@ describe("edit tool fuzzy matching", () => { // oldText with ASCII quotes should match const result = await editTool.execute("test-fuzzy-2", { path: testFile, - oldText: "console.log('hello');", - newText: "console.log('world');", + edits: [{ oldText: "console.log('hello');", newText: "console.log('world');" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -633,8 +612,7 @@ describe("edit tool fuzzy matching", () => { // oldText with ASCII quotes should match const result = await editTool.execute("test-fuzzy-3", { path: testFile, - oldText: 'const msg = "Hello World";', - newText: 'const msg = "Goodbye";', + edits: [{ oldText: 'const msg = "Hello World";', newText: 'const msg = "Goodbye";' }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -650,8 +628,7 @@ describe("edit tool fuzzy matching", () => { // oldText with ASCII hyphens should match const result = await editTool.execute("test-fuzzy-4", { path: testFile, - oldText: "range: 1-5\nbreak-here", - newText: "range: 10-50\nbreak--here", + edits: [{ oldText: "range: 1-5\nbreak-here", newText: "range: 10-50\nbreak--here" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -667,8 +644,7 @@ describe("edit tool fuzzy matching", () => { // oldText with regular space should match const result = await editTool.execute("test-fuzzy-5", { path: testFile, - oldText: "hello world", - newText: "hello universe", + edits: [{ oldText: "hello world", newText: "hello universe" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -683,8 +659,7 @@ describe("edit tool fuzzy matching", () => { const result = await editTool.execute("test-fuzzy-6", { path: testFile, - oldText: "const x = 'exact';", - newText: "const x = 'changed';", + edits: [{ oldText: "const x = 'exact';", newText: "const x = 'changed';" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -699,8 +674,7 @@ describe("edit tool fuzzy matching", () => { await expect( editTool.execute("test-fuzzy-7", { path: testFile, - oldText: "this does not exist", - newText: "replacement", + edits: [{ oldText: "this does not exist", newText: "replacement" }], }), ).rejects.toThrow(/Could not find the exact text/); }); @@ -713,8 +687,7 @@ describe("edit tool fuzzy matching", () => { await expect( editTool.execute("test-fuzzy-8", { path: testFile, - oldText: "hello world", - newText: "replaced", + edits: [{ oldText: "hello world", newText: "replaced" }], }), ).rejects.toThrow(/Found 2 occurrences/); }); @@ -754,8 +727,7 @@ describe("edit tool CRLF handling", () => { const result = await editTool.execute("test-crlf-1", { path: testFile, - oldText: "line two\n", - newText: "replaced line\n", + edits: [{ oldText: "line two\n", newText: "replaced line\n" }], }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -767,8 +739,7 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-crlf-2", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + edits: [{ oldText: "second\n", newText: "REPLACED\n" }], }); const content = readFileSync(testFile, "utf-8"); @@ -781,8 +752,7 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-lf-1", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + edits: [{ oldText: "second\n", newText: "REPLACED\n" }], }); const content = readFileSync(testFile, "utf-8"); @@ -797,8 +767,7 @@ describe("edit tool CRLF handling", () => { await expect( editTool.execute("test-crlf-dup", { path: testFile, - oldText: "hello\nworld\n", - newText: "replaced\n", + edits: [{ oldText: "hello\nworld\n", newText: "replaced\n" }], }), ).rejects.toThrow(/Found 2 occurrences/); }); @@ -809,8 +778,7 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-bom", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + edits: [{ oldText: "second\n", newText: "REPLACED\n" }], }); const content = readFileSync(testFile, "utf-8");