diff --git a/integration-tests/replace.test.ts b/integration-tests/replace.test.ts index e7ed23a2dc..0de23a04c7 100644 --- a/integration-tests/replace.test.ts +++ b/integration-tests/replace.test.ts @@ -61,4 +61,34 @@ describe('replace', () => { console.log('File replaced successfully. New content:', newFileContent); } }); + + it('should handle $ literally when replacing text ending with $', async () => { + const rig = new TestRig(); + await rig.setup( + 'should handle $ literally when replacing text ending with $', + ); + + const fileName = 'regex.yml'; + const originalContent = "| select('match', '^[sv]d[a-z]$')\n"; + const expectedContent = "| select('match', '^[sv]d[a-z]$') # updated\n"; + + rig.createFile(fileName, originalContent); + + const prompt = + "Open regex.yml and append ' # updated' after the line containing ^[sv]d[a-z]$ without breaking the $ character."; + + const result = await rig.run(prompt); + const foundToolCall = await rig.waitForToolCall('replace'); + + if (!foundToolCall) { + printDebugInfo(rig, result); + } + + expect(foundToolCall, 'Expected to find a replace tool call').toBeTruthy(); + + validateModelOutput(result, ['regex.yml'], 'Replace $ literal test'); + + const newFileContent = rig.readFile(fileName); + expect(newFileContent).toBe(expectedContent); + }); }); diff --git a/packages/a2a-server/src/agent/task.ts b/packages/a2a-server/src/agent/task.ts index f8c756992a..af65015687 100644 --- a/packages/a2a-server/src/agent/task.ts +++ b/packages/a2a-server/src/agent/task.ts @@ -509,7 +509,8 @@ export class Task { if (oldString === '' && !isNewFile) { return currentContent; } - return currentContent.replaceAll(oldString, newString); + // Use split/join to ensure replacement + return currentContent.split(oldString).join(newString); } async scheduleToolCalls( diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 7b98083e31..f3df1ead34 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -198,6 +198,22 @@ describe('EditTool', () => { 'hello world', ); }); + + it('should treat $ literally and not as replacement pattern', () => { + const current = "price is $100 and pattern end is ' '"; + const oldStr = 'price is $100'; + const newStr = 'price is $200'; + const result = applyReplacement(current, oldStr, newStr, false); + expect(result).toBe("price is $200 and pattern end is ' '"); + }); + + it("should treat $' literally and not as a replacement pattern", () => { + const current = 'foo'; + const oldStr = 'foo'; + const newStr = "bar$'baz"; + const result = applyReplacement(current, oldStr, newStr, false); + expect(result).toBe("bar$'baz"); + }); }); describe('validateToolParams', () => { diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index cb0418b1e4..6a2b58ae12 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -51,7 +51,8 @@ export function applyReplacement( if (oldString === '' && !isNewFile) { return currentContent; } - return currentContent.replaceAll(oldString, newString); + // Use split/join to ensure replacement + return currentContent.split(oldString).join(newString); } /** diff --git a/packages/core/src/tools/smart-edit.test.ts b/packages/core/src/tools/smart-edit.test.ts index 132d993306..bcad5b72ca 100644 --- a/packages/core/src/tools/smart-edit.test.ts +++ b/packages/core/src/tools/smart-edit.test.ts @@ -46,11 +46,11 @@ import { type Mock, } from 'vitest'; import { - applyReplacement, SmartEditTool, type EditToolParams, calculateReplacement, } from './smart-edit.js'; +import { applyReplacement } from './edit.js'; import { type FileDiff, ToolConfirmationOutcome } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import path from 'node:path'; @@ -169,6 +169,22 @@ describe('SmartEditTool', () => { 'hello new world new', ); }); + + it('should treat $ literally and not as replacement pattern', () => { + const current = 'regex end is $ and more'; + const oldStr = 'regex end is $'; + const newStr = 'regex end is $ and correct'; + const result = applyReplacement(current, oldStr, newStr, false); + expect(result).toBe('regex end is $ and correct and more'); + }); + + it("should treat $' literally and not as a replacement pattern", () => { + const current = 'foo'; + const oldStr = 'foo'; + const newStr = "bar$'baz"; + const result = applyReplacement(current, oldStr, newStr, false); + expect(result).toBe("bar$'baz"); + }); }); describe('calculateReplacement', () => { diff --git a/packages/core/src/tools/smart-edit.ts b/packages/core/src/tools/smart-edit.ts index 3647b73246..0f8089e7f8 100644 --- a/packages/core/src/tools/smart-edit.ts +++ b/packages/core/src/tools/smart-edit.ts @@ -30,26 +30,7 @@ import { } from './modifiable-tool.js'; import { IdeClient, IDEConnectionStatus } from '../ide/ide-client.js'; import { FixLLMEditWithInstruction } from '../utils/llm-edit-fixer.js'; - -export function applyReplacement( - currentContent: string | null, - oldString: string, - newString: string, - isNewFile: boolean, -): string { - if (isNewFile) { - return newString; - } - if (currentContent === null) { - // Should not happen if not a new file, but defensively return empty or newString if oldString is also empty - return oldString === '' ? newString : ''; - } - // If oldString is empty and it's not a new file, do not modify the content. - if (oldString === '' && !isNewFile) { - return currentContent; - } - return currentContent.replaceAll(oldString, newString); -} +import { applyReplacement } from './edit.js'; interface ReplacementContext { params: EditToolParams;