From 0e284457be45638a84c110444c0cea17678c6460 Mon Sep 17 00:00:00 2001
From: Gal Zahavi <38544478+galz10@users.noreply.github.com>
Date: Fri, 5 Sep 2025 13:34:02 -0700
Subject: [PATCH] Revert "Fix dollar sign replacement bug in file editing
 (#7703)" (#7823)

---
 integration-tests/replace.test.ts          | 30 ----------------------
 packages/a2a-server/src/agent/task.ts      |  3 +--
 packages/core/src/tools/edit.test.ts       | 16 ------------
 packages/core/src/tools/edit.ts            |  3 +--
 packages/core/src/tools/smart-edit.test.ts | 18 +------------
 packages/core/src/tools/smart-edit.ts      | 21 ++++++++++++++-
 6 files changed, 23 insertions(+), 68 deletions(-)

diff --git a/integration-tests/replace.test.ts b/integration-tests/replace.test.ts
index 0de23a04c7..e7ed23a2dc 100644
--- a/integration-tests/replace.test.ts
+++ b/integration-tests/replace.test.ts
@@ -61,34 +61,4 @@ 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 af65015687..f8c756992a 100644
--- a/packages/a2a-server/src/agent/task.ts
+++ b/packages/a2a-server/src/agent/task.ts
@@ -509,8 +509,7 @@ export class Task {
     if (oldString === '' && !isNewFile) {
       return currentContent;
     }
-    // Use split/join to ensure replacement
-    return currentContent.split(oldString).join(newString);
+    return currentContent.replaceAll(oldString, newString);
   }
 
   async scheduleToolCalls(
diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts
index f3df1ead34..7b98083e31 100644
--- a/packages/core/src/tools/edit.test.ts
+++ b/packages/core/src/tools/edit.test.ts
@@ -198,22 +198,6 @@ 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 6a2b58ae12..cb0418b1e4 100644
--- a/packages/core/src/tools/edit.ts
+++ b/packages/core/src/tools/edit.ts
@@ -51,8 +51,7 @@ export function applyReplacement(
   if (oldString === '' && !isNewFile) {
     return currentContent;
   }
-  // Use split/join to ensure replacement
-  return currentContent.split(oldString).join(newString);
+  return currentContent.replaceAll(oldString, newString);
 }
 
 /**
diff --git a/packages/core/src/tools/smart-edit.test.ts b/packages/core/src/tools/smart-edit.test.ts
index bcad5b72ca..132d993306 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,22 +169,6 @@ 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 0f8089e7f8..3647b73246 100644
--- a/packages/core/src/tools/smart-edit.ts
+++ b/packages/core/src/tools/smart-edit.ts
@@ -30,7 +30,26 @@ import {
 } from './modifiable-tool.js';
 import { IdeClient, IDEConnectionStatus } from '../ide/ide-client.js';
 import { FixLLMEditWithInstruction } from '../utils/llm-edit-fixer.js';
-import { applyReplacement } from './edit.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);
+}
 
 interface ReplacementContext {
   params: EditToolParams;