fix(filesystem): harden file replacement with EPERM fallback

src/filesystem/__tests__/lib.test.ts

@@ -308,6 +308,61 @@ describe('Lib Functions', () => {
 
         expect(mockFs.writeFile).toHaveBeenCalledWith('/test/file.txt', 'new content', { encoding: "utf-8", flag: 'wx' });
       });
+
+      it('falls back to fs.cp when fs.rename fails with EPERM', async () => {
+        const eexistError = new Error('EEXIST') as NodeJS.ErrnoException;
+        eexistError.code = 'EEXIST';
+        const epermError = new Error('EPERM') as NodeJS.ErrnoException;
+        epermError.code = 'EPERM';
+
+        mockFs.writeFile
+          .mockRejectedValueOnce(eexistError)   // First write fails (file exists)
+          .mockResolvedValueOnce(undefined);     // Temp file write succeeds
+        mockFs.rename.mockRejectedValueOnce(epermError);  // Rename fails (locked)
+        mockFs.cp.mockResolvedValueOnce(undefined);       // cp succeeds
+        mockFs.unlink.mockResolvedValueOnce(undefined);   // Temp cleanup succeeds
+
+        await writeFileContent('/test/file.txt', 'new content');
+
+        expect(mockFs.rename).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt'
+        );
+        expect(mockFs.cp).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt',
+          { force: true }
+        );
+        expect(mockFs.unlink).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/)
+        );
+      });
+
+      it('succeeds when fs.cp works but temp file unlink fails', async () => {
+        const eexistError = new Error('EEXIST') as NodeJS.ErrnoException;
+        eexistError.code = 'EEXIST';
+        const epermError = new Error('EPERM') as NodeJS.ErrnoException;
+        epermError.code = 'EPERM';
+        const ebusyError = new Error('EBUSY') as NodeJS.ErrnoException;
+        ebusyError.code = 'EBUSY';
+
+        mockFs.writeFile
+          .mockRejectedValueOnce(eexistError)
+          .mockResolvedValueOnce(undefined);
+        mockFs.rename.mockRejectedValueOnce(epermError);
+        mockFs.cp.mockResolvedValueOnce(undefined);
+        mockFs.unlink.mockRejectedValueOnce(ebusyError);   // Temp cleanup fails (e.g. antivirus)
+
+        // Should NOT throw — the target file was written successfully
+        await expect(writeFileContent('/test/file.txt', 'new content'))
+          .resolves.toBeUndefined();
+
+        expect(mockFs.cp).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt',
+          { force: true }
+        );
+      });
     });
 
   });
@@ -553,6 +608,61 @@ describe('Lib Functions', () => {
         );
       });
 
+      it('falls back to fs.cp when fs.rename fails with EPERM during file edit', async () => {
+        const epermError = new Error('EPERM') as NodeJS.ErrnoException;
+        epermError.code = 'EPERM';
+
+        mockFs.readFile.mockResolvedValue('line1\nline2\nline3\n');
+        mockFs.writeFile.mockResolvedValue(undefined);
+        mockFs.rename.mockRejectedValueOnce(epermError);
+        mockFs.cp.mockResolvedValueOnce(undefined);
+        mockFs.unlink.mockResolvedValueOnce(undefined);
+
+        const edits = [{ oldText: 'line2', newText: 'modified line2' }];
+        const result = await applyFileEdits('/test/file.txt', edits, false);
+
+        // Should have tried rename first, then fallen back to cp + unlink
+        expect(mockFs.rename).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt'
+        );
+        expect(mockFs.cp).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt',
+          { force: true }
+        );
+        expect(mockFs.unlink).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/)
+        );
+        // Edit should still produce a valid diff
+        expect(result).toContain('modified line2');
+      });
+
+      it('succeeds when fs.cp works but temp file unlink fails during file edit', async () => {
+        const epermError = new Error('EPERM') as NodeJS.ErrnoException;
+        epermError.code = 'EPERM';
+        const ebusyError = new Error('EBUSY') as NodeJS.ErrnoException;
+        ebusyError.code = 'EBUSY';
+
+        mockFs.readFile.mockResolvedValue('line1\nline2\nline3\n');
+        mockFs.writeFile.mockResolvedValue(undefined);
+        mockFs.rename.mockRejectedValueOnce(epermError);
+        mockFs.cp.mockResolvedValueOnce(undefined);
+        mockFs.unlink.mockRejectedValueOnce(ebusyError);  // Temp cleanup fails
+
+        const edits = [{ oldText: 'line2', newText: 'modified line2' }];
+
+        // Should NOT throw — the target file was written successfully
+        const result = await applyFileEdits('/test/file.txt', edits, false);
+
+        expect(result).toContain('modified line2');
+        expect(mockFs.cp).toHaveBeenCalledWith(
+          expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/),
+          '/test/file.txt',
+          { force: true }
+        );
+      });
+
       it('handles CRLF line endings in file content', async () => {
         mockFs.readFile.mockResolvedValue('line1\r\nline2\r\nline3\r\n');
 

src/filesystem/lib.ts

@@ -140,6 +140,47 @@ export async function validatePath(requestedPath: string): Promise<string> {
 }
 
 
+/**
+ * Replace a target file with the contents of a temporary file.
+ *
+ * Uses `fs.rename` for an atomic swap when possible. On Windows, rename can
+ * fail with `EPERM` when the target is held open by another process (e.g.
+ * VS Code) *provided* that process opened the file with `FILE_SHARE_DELETE`.
+ * In that case the function falls back to `fs.cp` + best-effort `fs.unlink`.
+ *
+ * **Limitations:**
+ * - The `fs.cp` fallback is *not* atomic — there is a brief window between
+ *   the internal `unlink(dest)` and `copyFile(src, dest)` performed by
+ *   `fs.cp({ force: true })`.
+ * - The fallback only succeeds when the locking process uses
+ *   `FILE_SHARE_DELETE`. Editors that lock without this flag will still
+ *   produce an `EPERM` error.
+ *
+ * @param tempPath  Path to the temporary file that contains the new content.
+ * @param targetPath  Path to the destination file to be replaced.
+ */
+async function replaceFileFromTemp(tempPath: string, targetPath: string): Promise<void> {
+  try {
+    await fs.rename(tempPath, targetPath);
+  } catch (renameError) {
+    if ((renameError as NodeJS.ErrnoException).code === 'EPERM') {
+      // Fallback: copy then best-effort cleanup
+      await fs.cp(tempPath, targetPath, { force: true });
+      try {
+        await fs.unlink(tempPath);
+      } catch {
+        // Best-effort cleanup; target was already written successfully
+      }
+    } else {
+      // For non-EPERM errors, clean up the temp file and re-throw
+      try {
+        await fs.unlink(tempPath);
+      } catch {}
+      throw renameError;
+    }
+  }
+}
+
 // File Operations
 export async function getFileStats(filePath: string): Promise<FileInfo> {
   const stats = await fs.stat(filePath);
@@ -169,15 +210,8 @@ export async function writeFileContent(filePath: string, content: string): Promi
       // could be created between validation and write. Rename operations
       // replace the target file atomically and don't follow symlinks.
       const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`;
-      try {
-        await fs.writeFile(tempPath, content, 'utf-8');
-        await fs.rename(tempPath, filePath);
-      } catch (renameError) {
-        try {
-          await fs.unlink(tempPath);
-        } catch {}
-        throw renameError;
-      }
+      await fs.writeFile(tempPath, content, 'utf-8');
+      await replaceFileFromTemp(tempPath, filePath);
     } else {
       throw error;
     }
@@ -267,15 +301,8 @@ export async function applyFileEdits(
     // could be created between validation and write. Rename operations
     // replace the target file atomically and don't follow symlinks.
     const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`;
-    try {
-      await fs.writeFile(tempPath, modifiedContent, 'utf-8');
-      await fs.rename(tempPath, filePath);
-    } catch (error) {
-      try {
-        await fs.unlink(tempPath);
-      } catch {}
-      throw error;
-    }
+    await fs.writeFile(tempPath, modifiedContent, 'utf-8');
+    await replaceFileFromTemp(tempPath, filePath);
   }
 
   return formattedDiff;