fix(chat): Add more fsWrite validation and validate before diff view

Author: jguoamz

packages/core/src/codewhispererChat/controllers/chat/messenger/messenger.ts

@@ -331,6 +331,8 @@ export class Messenger {
                                         explanation = input.explanation
                                     }
                                     session.setShowDiffOnFileWrite(true)
+                                    // validate the fsWrite tool input before showing the diff view
+                                    await tool.tool.validate()
                                     changeList = await tool.tool.getDiffChanges()
                                 }
                                 if (tool.type === ToolType.FsRead) {

packages/core/src/codewhispererChat/tools/fsWrite.ts

@@ -17,8 +17,7 @@ interface BaseParams {
 
 export interface CreateParams extends BaseParams {
     command: 'create'
-    fileText?: string
-    newStr?: string
+    fileText: string
 }
 
 export interface StrReplaceParams extends BaseParams {
@@ -118,24 +117,42 @@ export class FsWrite {
     }
 
     public async validate(): Promise<void> {
+        if (!this.params.path) {
+            throw new Error('Path must not be empty')
+        }
+        const sanitizedPath = sanitizePath(this.params.path)
         switch (this.params.command) {
-            case 'create':
-                if (!this.params.path) {
-                    throw new Error('Path must not be empty')
+            case 'create': {
+                if (this.params.fileText === undefined) {
+                    throw new Error('fileText must be provided for create command')
+                }
+                const fileExists = await fs.existsFile(sanitizedPath)
+                if (fileExists) {
+                    const oldContent = await fs.readFileText(sanitizedPath)
+                    if (oldContent === this.params.fileText) {
+                        throw new Error('The file already exists with the same content')
+                    }
                 }
                 break
-            case 'strReplace':
+            }
+            case 'strReplace': {
+                if (this.params.oldStr === this.params.newStr) {
+                    throw new Error('The provided oldStr and newStr are the exact same, this is a no-op')
+                }
+                const fileExists = await fs.existsFile(sanitizedPath)
+                if (!fileExists) {
+                    throw new Error('The provided path must exist in order to replace contents into it')
+                }
+                break
+            }
             case 'insert': {
-                const fileExists = await fs.existsFile(this.params.path)
+                const fileExists = await fs.existsFile(sanitizedPath)
                 if (!fileExists) {
-                    throw new Error('The provided path must exist in order to replace or insert contents into it')
+                    throw new Error('The provided path must exist in order to insert contents into it')
                 }
                 break
             }
             case 'append':
-                if (!this.params.path) {
-                    throw new Error('Path must not be empty')
-                }
                 if (!this.params.newStr) {
                     throw new Error('Content to append must not be empty')
                 }
@@ -220,15 +237,7 @@ export class FsWrite {
     }
 
     private getCreateCommandText(params: CreateParams): string {
-        if (params.fileText) {
-            return params.fileText
-        }
-        if (params.newStr) {
-            this.logger.warn('Required field `fileText` is missing, use the provided `newStr` instead')
-            return params.newStr
-        }
-        this.logger.warn('No content provided for the create command')
-        return ''
+        return params.fileText
     }
 
     private escapeRegExp(string: string): string {

packages/core/src/test/codewhispererChat/tools/fsWrite.test.ts

@@ -66,39 +66,6 @@ describe('FsWrite Tool', function () {
 
             assert.deepStrictEqual(output, expectedOutput)
         })
-
-        it('uses newStr when fileText is not provided', async function () {
-            const filePath = path.join(testFolder.path, 'file2.txt')
-
-            const params: CreateParams = {
-                command: 'create',
-                newStr: 'Hello World',
-                path: filePath,
-            }
-            const fsWrite = new FsWrite(params)
-            const output = await fsWrite.invoke(process.stdout)
-
-            const content = await fs.readFileText(filePath)
-            assert.strictEqual(content, 'Hello World')
-
-            assert.deepStrictEqual(output, expectedOutput)
-        })
-
-        it('creates an empty file when no content is provided', async function () {
-            const filePath = path.join(testFolder.path, 'file3.txt')
-
-            const params: CreateParams = {
-                command: 'create',
-                path: filePath,
-            }
-            const fsWrite = new FsWrite(params)
-            const output = await fsWrite.invoke(process.stdout)
-
-            const content = await fs.readFileText(filePath)
-            assert.strictEqual(content, '')
-
-            assert.deepStrictEqual(output, expectedOutput)
-        })
     })
 
     describe('handleStrReplace', async function () {