From b0ec9aee8bfb64dcb7dd4f540e23e065e997c18e Mon Sep 17 00:00:00 2001 From: aditya Date: Fri, 22 May 2026 02:25:45 +0530 Subject: [PATCH] fix(filesystem): preserve literal $ in edit_file newText (closes #4157) The prior code passed `normalizedNew` as String.prototype.replace's replacement argument. JS interprets `$$`, `$&`, `$'`, `$``, and `$n` as replacement patterns in that form, so any actual `$` characters in the user's newText - price tags, regex backreferences in docs, shell examples - get expanded or dropped. The callback form (`replace(..., () => string)`) disables pattern interpretation, so the string is written verbatim. Two regression tests pin the callback form. Closes #4157. --- src/filesystem/__tests__/lib.test.ts | 34 ++++++++++++++++++++++++++++ src/filesystem/lib.ts | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index f7e585af22..05d4ea5fb1 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -440,6 +440,40 @@ describe('Lib Functions', () => { ); }); + it("preserves literal $ characters in newText (regression for #4157)", async () => { + // String.prototype.replace interprets $$, $&, $`, $' in the + // replacement argument when it is a string. The callback form + // disables that interpretation. This test pins the callback form + // so the bug can't silently regress. + mockFs.readFile.mockResolvedValue("price: PLACEHOLDER\n"); + const edits = [ + { oldText: "PLACEHOLDER", newText: "$$100 USD" } + ]; + mockFs.rename.mockResolvedValueOnce(undefined); + await applyFileEdits("/test/file.txt", edits, false); + expect(mockFs.writeFile).toHaveBeenCalledWith( + expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/), + "price: $$100 USD\n", + "utf-8" + ); + }); + + it("preserves $&, $` and $' replacement-pattern tokens in newText (#4157)", async () => { + mockFs.readFile.mockResolvedValue("TARGET\n"); + const edits = [ + { oldText: "TARGET", newText: "a $& b $` c $' d" } + ]; + mockFs.rename.mockResolvedValueOnce(undefined); + await applyFileEdits("/test/file.txt", edits, false); + // Without the fix: $& would expand to "TARGET", $` to "" (before-match), + // and $' to "" (after-match), corrupting the output. + expect(mockFs.writeFile).toHaveBeenCalledWith( + expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/), + "a $& b $` c $' d\n", + "utf-8" + ); + }); + it('handles dry run mode', async () => { const edits = [ { oldText: 'line2', newText: 'modified line2' } diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 17e4654cd5..ce4af9f38a 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -207,7 +207,7 @@ export async function applyFileEdits( // If exact match exists, use it if (modifiedContent.includes(normalizedOld)) { - modifiedContent = modifiedContent.replace(normalizedOld, normalizedNew); + modifiedContent = modifiedContent.replace(normalizedOld, () => normalizedNew); continue; }