From c5137fbdd0cab3e84e3d71fbdc0fc646fdcbf2ce Mon Sep 17 00:00:00 2001 From: MattBenesch <138265545+MattBenesch@users.noreply.github.com> Date: Fri, 6 Feb 2026 18:19:31 -0600 Subject: [PATCH] fix(filesystem): handle Windows EPERM when renaming over locked files On Windows, fs.rename() fails with EPERM when the target file is locked by another process (e.g., open in an editor). This adds a fallback that uses fs.cp() + fs.unlink() when EPERM is encountered, allowing edits to succeed even when the file is open. Fixes #3199 --- src/filesystem/__tests__/lib.test.ts | 59 +++++++++++++++++++++++++++- src/filesystem/lib.ts | 29 ++++++++++++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index bfe8987bfd..e6a23dd872 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -279,11 +279,66 @@ describe('Lib Functions', () => { describe('writeFileContent', () => { it('writes file content', async () => { mockFs.writeFile.mockResolvedValueOnce(undefined); - + await writeFileContent('/test/file.txt', 'new content'); - + 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 (Windows locked file)', async () => { + // First write fails because file exists + const eexistError = new Error('EEXIST') as NodeJS.ErrnoException; + eexistError.code = 'EEXIST'; + + // Rename fails with EPERM (Windows file lock) + 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); + mockFs.cp.mockResolvedValueOnce(undefined); + mockFs.unlink.mockResolvedValueOnce(undefined); + + await writeFileContent('/test/file.txt', 'new content'); + + // 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$/) + ); + }); + + it('propagates non-EPERM errors from fs.rename', async () => { + // First write fails because file exists + const eexistError = new Error('EEXIST') as NodeJS.ErrnoException; + eexistError.code = 'EEXIST'; + + // Rename fails with EACCES (not EPERM) + const eaccesError = new Error('EACCES') as NodeJS.ErrnoException; + eaccesError.code = 'EACCES'; + + mockFs.writeFile + .mockRejectedValueOnce(eexistError) + .mockResolvedValueOnce(undefined); + mockFs.rename.mockRejectedValueOnce(eaccesError); + mockFs.unlink.mockResolvedValueOnce(undefined); // Cleanup of temp file + + await expect(writeFileContent('/test/file.txt', 'content')) + .rejects.toThrow('EACCES'); + + // Should not have tried cp fallback + expect(mockFs.cp).not.toHaveBeenCalled(); + }); }); }); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 240ca0d476..b50da0cf12 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -135,6 +135,27 @@ export async function readFileContent(filePath: string, encoding: string = 'utf- return await fs.readFile(filePath, encoding as BufferEncoding); } +/** + * Atomically replaces a file by renaming a temporary file over it. + * On Windows, fs.rename() fails with EPERM when the target file is locked + * by another process (e.g., open in an editor). This function falls back + * to fs.cp() + fs.unlink() which can overwrite locked files. + */ +async function atomicReplaceFile(tempPath: string, targetPath: string): Promise { + try { + await fs.rename(tempPath, targetPath); + } catch (error) { + // On Windows, rename fails with EPERM if target file is locked by another process + if ((error as NodeJS.ErrnoException).code === 'EPERM') { + // Fallback: copy over the locked file, then remove temp file + await fs.cp(tempPath, targetPath, { force: true }); + await fs.unlink(tempPath); + } else { + throw error; + } + } +} + export async function writeFileContent(filePath: string, content: string): Promise { try { // Security: 'wx' flag ensures exclusive creation - fails if file/symlink exists, @@ -148,12 +169,12 @@ export async function writeFileContent(filePath: string, content: string): Promi const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, content, 'utf-8'); - await fs.rename(tempPath, filePath); - } catch (renameError) { + await atomicReplaceFile(tempPath, filePath); + } catch (replaceError) { try { await fs.unlink(tempPath); } catch {} - throw renameError; + throw replaceError; } } else { throw error; @@ -246,7 +267,7 @@ export async function applyFileEdits( const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`; try { await fs.writeFile(tempPath, modifiedContent, 'utf-8'); - await fs.rename(tempPath, filePath); + await atomicReplaceFile(tempPath, filePath); } catch (error) { try { await fs.unlink(tempPath);