fix: update toolSpec, add special character handling/escaped characters

Author: jguoamz

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsReplace.test.ts

@@ -15,7 +15,7 @@ describe('FsReplace Tool', function () {
     const expectedOutput: InvokeOutput = {
         output: {
             kind: 'text',
-            content: '',
+            content: 'File updated successfully',
         },
     }
 

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsReplace.ts

@@ -36,7 +36,7 @@ export class FsReplace {
         const sanitizedPath = sanitize(params.path)
         for (const diff of params.diffs) {
             if (diff.oldStr === diff.newStr) {
-                throw new Error('The provided oldStr and newStr are the exact same, this is a no-op')
+                throw new Error(`The provided oldStr and newStr "${diff.oldStr}" are the exact same, this is a no-op`)
             }
             const fileExists = await this.workspace.fs.exists(sanitizedPath)
             if (!fileExists) {
@@ -53,7 +53,7 @@ export class FsReplace {
         return {
             output: {
                 kind: 'text',
-                content: '',
+                content: 'File updated successfully',
             },
         }
     }
@@ -74,19 +74,20 @@ export class FsReplace {
             description:
                 'A tool for search and replace contents of an existing file.\n\n' +
                 '## Overview\n' +
-                'This tool replaces sections of content in an existing file using oldStr/newStr blocks that define exact changes to specific parts of the file.\n\n' +
+                'This tool replaces sections of content in an existing file using `oldStr`/`newStr` blocks that define exact changes to specific parts of the file. You MUST ALWAYS bundle as many changes as you can by populating the diffs array with different `oldStr`/`newStr` pairs. You MUST ALWAYS read the contents of the file you want to update first before using the tool to ensure successful result.\n\n' +
                 '## When to use\n' +
                 '- When you need to make targeted changes to specific parts of a file\n' +
                 '- When you need to update multiple sections of the same file\n' +
                 '## When not to use\n' +
+                '- When `oldStr` and `newStr` are identical, as the tool will throw an error for no-op replacements\n' +
                 '- When you need to create a new file\n' +
                 '- When you need to rename or move a file\n\n' +
                 '## IMPORTANT Notes\n' +
-                '- Use this tool to delete code by using empty `newStr` parameter.\n' +
-                '- The `oldStr` parameter should match EXACTLY one or more consecutive lines from the original file. Be mindful of whitespaces! Include just the changing lines, and a few surrounding lines if needed for uniqueness. Do not include long runs of unchanging lines in `oldStr`.\n' +
-                '- The `newStr` parameter should contain the edited lines that should replace the `oldStr`.\n' +
-                '- When multiple edits to the same file are needed, populate the diffs array with `oldStr` and `newStr` pairs. This improves efficiency by reducing the number of tool calls and ensures the file remains in a consistent state.\n' +
-                '- Each `oldStr` must be unique within the file - if there are multiple matches, the operation will fail.',
+                '- You MUST ALWAYS read the contents of the file you want to update first before using the tool to ensure successful result\n' +
+                '- Use this tool to delete code by using empty `newStr` parameter\n' +
+                '- The `oldStr` parameter should match EXACTLY one or more consecutive lines from the original file. Be mindful of whitespaces! Include just the changing lines, and a few surrounding lines if needed for uniqueness. Do not include long runs of unchanging lines in `oldStr`\n' +
+                '- The `newStr` parameter should contain the edited lines that should replace the `oldStr`\n' +
+                '- When multiple edits to the same file are needed, ALWAYS populate the diffs array with `oldStr` and `newStr` pairs. This improves efficiency by reducing the number of tool calls and ensures the file remains in a consistent state',
             inputSchema: {
                 type: 'object',
                 properties: {
@@ -96,7 +97,8 @@ export class FsReplace {
                         type: 'string',
                     },
                     diffs: {
-                        description: 'List of diffs to replace contents in an existing file.',
+                        description:
+                            'List of diffs to replace contents in an existing file. For example, `[{"oldStr": "existingContent", "newStr": "newContent"}]`',
                         type: 'array',
                         items: {
                             type: 'object',
@@ -136,23 +138,50 @@ const getReplaceContent = (params: ReplaceParams, fileContent: string) => {
         if (diff.newStr == undefined) {
             diff.newStr = ''
         }
+
         // Normalize oldStr and newStr to match fileContent's line ending style
         const normalizedOldStr = diff.oldStr.split(/\r\n|\r|\n/).join(lineEnding)
         const normalizedNewStr = diff.newStr.split(/\r\n|\r|\n/).join(lineEnding)
 
-        const matches = [...fileContent.matchAll(new RegExp(escapeRegExp(normalizedOldStr), 'g'))]
+        const unescapedOldStr = unescapeString(normalizedOldStr)
+        const unescapedNewStr = unescapeString(normalizedNewStr)
+
+        // Use string indexOf and substring for safer replacement with special characters
+        const startIndex = fileContent.indexOf(unescapedOldStr)
 
-        if (matches.length === 0) {
+        if (startIndex === -1) {
             throw new Error(`No occurrences of "${diff.oldStr}" were found`)
         }
-        if (matches.length > 1) {
-            throw new Error(`${matches.length} occurrences of oldStr were found when only 1 is expected`)
+
+        // Check for multiple occurrences
+        const secondIndex = fileContent.indexOf(unescapedOldStr, startIndex + 1)
+        if (secondIndex !== -1) {
+            throw new Error(`Multiple occurrences of "${diff.oldStr}" were found when only 1 is expected`)
         }
-        fileContent = fileContent.replace(normalizedOldStr, normalizedNewStr)
+
+        // Perform the replacement using string operations instead of regex
+        fileContent =
+            fileContent.substring(0, startIndex) +
+            unescapedNewStr +
+            fileContent.substring(startIndex + unescapedOldStr.length)
     }
     return fileContent
 }
 
-const escapeRegExp = (string: string) => {
-    return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+// Handle all standard JavaScript escape sequences
+const unescapeString = (str: string): string => {
+    return str
+        .replace(/\\t/g, '\t') // Tab
+        .replace(/\\n/g, '\n') // Line feed
+        .replace(/\\r/g, '\r') // Carriage return
+        .replace(/\\\\/g, '\\') // Backslash
+        .replace(/\\"/g, '"') // Double quote
+        .replace(/\\'/g, "'") // Single quote
+        .replace(/\\b/g, '\b') // Backspace
+        .replace(/\\f/g, '\f') // Form feed
+        .replace(/\\v/g, '\v') // Vertical tab
+        .replace(/\\0/g, '\0') // Null character
+        .replace(/\\\//g, '/') // Forward slash
+        .replace(/\\u([0-9a-fA-F]{4})/g, (_, hex) => String.fromCharCode(parseInt(hex, 16))) // Unicode escape sequences
+        .replace(/\\x([0-9a-fA-F]{2})/g, (_, hex) => String.fromCharCode(parseInt(hex, 16))) // Hexadecimal escape sequences
 }

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts

@@ -14,7 +14,13 @@ describe('FsWrite Tool', function () {
     const expectedOutput: InvokeOutput = {
         output: {
             kind: 'text',
-            content: '',
+            content: 'File created successfully',
+        },
+    }
+    const expectedOutputAppend: InvokeOutput = {
+        output: {
+            kind: 'text',
+            content: 'File appended successfully',
         },
     }
 
@@ -115,7 +121,7 @@ describe('FsWrite Tool', function () {
             const newContent = await features.workspace.fs.readFile(filePath)
             assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4')
 
-            assert.deepStrictEqual(output, expectedOutput)
+            assert.deepStrictEqual(output, expectedOutputAppend)
         })
 
         it('adds a newline before appending if file does not end with one', async function () {
@@ -134,7 +140,7 @@ describe('FsWrite Tool', function () {
             const newContent = await features.workspace.fs.readFile(filePath)
             assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4')
 
-            assert.deepStrictEqual(output, expectedOutput)
+            assert.deepStrictEqual(output, expectedOutputAppend)
         })
 
         it('appends to an empty file', async function () {
@@ -152,7 +158,7 @@ describe('FsWrite Tool', function () {
             const newContent = await features.workspace.fs.readFile(filePath)
             assert.strictEqual(newContent, 'Line 1')
 
-            assert.deepStrictEqual(output, expectedOutput)
+            assert.deepStrictEqual(output, expectedOutputAppend)
         })
 
         it('appends multiple lines correctly', async function () {
@@ -169,7 +175,7 @@ describe('FsWrite Tool', function () {
             const newContent = await features.workspace.fs.readFile(filePath)
             assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3')
 
-            assert.deepStrictEqual(output, expectedOutput)
+            assert.deepStrictEqual(output, expectedOutputAppend)
         })
 
         it('throws error when file does not exist', async function () {

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts

@@ -63,20 +63,22 @@ export class FsWrite {
 
     public async invoke(params: FsWriteParams): Promise<InvokeOutput> {
         const sanitizedPath = sanitize(params.path)
-
+        let content = ''
         switch (params.command) {
             case 'create':
                 await this.handleCreate(params, sanitizedPath)
+                content = 'File created successfully'
                 break
             case 'append':
                 await this.handleAppend(params, sanitizedPath)
+                content = 'File appended successfully'
                 break
         }
 
         return {
             output: {
                 kind: 'text',
-                content: '',
+                content,
             },
         }
     }