fix(filesystem): harden file replacement with EPERM fallback

[Chelebii]

Problem

When \s.rename\ fails with \EPERM\ on Windows (e.g. VS Code or antivirus holding a file lock), both \writeFileContent\ and \�pplyFileEdits\ propagate the error even though the write could succeed via a fallback path. Additionally, if the fallback copy succeeds but temp file cleanup fails (antivirus briefly locks the temp), the user sees an error despite the target file being written correctly.

Ref: #3430, #3199

What changed

• Extracted
  eplaceFileFromTemp\ helper from the inline rename logic in \writeFileContent\ and \�pplyFileEdits, eliminating code duplication.
• Added EPERM fallback: when \s.rename\ fails with \EPERM, falls back to \s.cp({ force: true })\ + best-effort \s.unlink\ of the temp file.
• Best-effort temp cleanup: if \s.cp\ succeeds but \s.unlink(tempPath)\ fails, the error is swallowed — the target was already written successfully.
• JSDoc documentation: documents \FILE_SHARE_DELETE\ limitation and the non-atomic nature of the \s.cp\ fallback.
• 5 new tests covering:
  • EPERM fallback through \writeFileContent\
  • EPERM fallback through \�pplyFileEdits\
  • Successful write despite temp cleanup failure (\writeFileContent)
  • Successful edit despite temp cleanup failure (\�pplyFileEdits)
  • Existing behavior preserved (non-EPERM errors still propagate)

Validation

\
npx vitest run
✓ tests/lib.test.ts (49 tests)
✓ tests/path-utils.test.ts (29 tests)
✓ tests/roots-utils.test.ts (3 tests)
✓ tests/path-validation.test.ts (53 tests)
✓ tests/directory-tree.test.ts (7 tests)
✓ tests/structured-content.test.ts (5 tests)
✓ tests/startup-validation.test.ts (4 tests)

Test Files 7 passed (7)
Tests 150 passed (150)
\\

\ sc --noEmit\ — 0 errors.

Checklist (from #3430)

• Temp file cleanup is best-effort (no false errors on successful writes)
• JSDoc documents \FILE_SHARE_DELETE\ requirement and non-atomic fallback
• \�pplyFileEdits\ has EPERM fallback test coverage
• Function name accurately reflects behavior (
  eplaceFileFromTemp)
• All acceptance tests pass

Fixes #3430

[Chelebii]

Follow-up: all checks are now passing after the lockfile refresh commit (c22fad2). The earlier Python failure was caused by src/fetch/uv.lock being out of sync with src/fetch/pyproject.toml, and that is now resolved.\n\nAt this point the PR looks blocked only on maintainer review / merge, not on CI.

[Chelebii]

@codex review focus on bugs, edge cases and missing tests

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 49s —— View job

---

Code Review

• Read changed files (lib.ts, lib.test.ts)
• Analyze implementation correctness and edge cases
• Check test coverage
• Post review feedback

---

Overall this is a clean, well-motivated change. The refactoring reduces duplication, the JSDoc is excellent, and the new tests cover the primary scenarios. There are a couple of real issues worth addressing before merge.

---

Bug: orphaned temp file when fs.cp itself fails

src/filesystem/lib.ts lines 166–181 — when fs.rename fails with EPERM and the fallback fs.cp also throws (e.g. disk full, target directory removed), the temp file is never cleaned up:

if ((renameError as NodeJS.ErrnoException).code === 'EPERM') {
  await fs.cp(tempPath, targetPath, { force: true }); // throws → temp file orphaned
  try { await fs.unlink(tempPath); } catch {}          // never reached
}

The caller (writeFileContent, applyFileEdits) has no finally block to clean up tempPath, so the *.hex.tmp file is left on disk. The fix is to catch the cp error, attempt cleanup, then re-throw:

if ((renameError as NodeJS.ErrnoException).code === 'EPERM') {
  try {
    await fs.cp(tempPath, targetPath, { force: true });
  } catch (cpError) {
    try { await fs.unlink(tempPath); } catch {}
    throw cpError;
  }
  try { await fs.unlink(tempPath); } catch {}
}

Fix this →

---

Minor regression: writeFile(tempPath) failure no longer attempts cleanup

Previously, the temp-file write and rename were in a single try block, so even a failed writeFile(tempPath, ...) triggered unlink(tempPath). With the refactor, if fs.writeFile(tempPath, ...) throws (e.g. quota exceeded mid-write), replaceFileFromTemp is never called and the partial temp file is orphaned. The old behaviour:

// OLD — unlink is attempted even if writeFile fails
try {
  await fs.writeFile(tempPath, content, 'utf-8');
  await fs.rename(tempPath, filePath);
} catch {
  try { await fs.unlink(tempPath); } catch {}
  throw;
}

A simple fix is a try/finally (or try/catch) around the write+replace pair in both call sites:

try {
  await fs.writeFile(tempPath, content, 'utf-8');
  await replaceFileFromTemp(tempPath, filePath);
} catch (err) {
  try { await fs.unlink(tempPath); } catch {}
  throw err;
}

Fix this →

---

Missing test: fs.cp failure in EPERM path

There is no test verifying that a fs.cp error (after an EPERM rename) propagates to the caller and that the temp file is cleaned up. Given the orphan-file bug above, a test like this would catch regressions:

it('cleans up temp and re-throws when fs.cp fails after EPERM', async () => {
  const epermError = Object.assign(new Error('EPERM'), { code: 'EPERM' });
  const enospcError = Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' });

  mockFs.writeFile.mockResolvedValueOnce(undefined);
  mockFs.rename.mockRejectedValueOnce(epermError);
  mockFs.cp.mockRejectedValueOnce(enospcError);
  mockFs.unlink.mockResolvedValueOnce(undefined);

  await expect(writeFileContent('/tmp/file.txt', 'x')).rejects.toThrow('ENOSPC');
  expect(mockFs.unlink).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/));
});

---

Minor notes (no action required)

• EACCES on Linux/macOS: File-lock scenarios on Unix manifest as EACCES, not EPERM. The fallback won't help there, but the JSDoc clearly scopes this to the Windows FILE_SHARE_DELETE case, so this is acceptable as-is.
• fs.cp vs fs.copyFile: For a single file, fs.copyFile is more direct, but fs.cp({ force: true }) is fine for Node 22 and handles the overwrite semantic correctly.
• The code deduplication is clean and the JSDoc comments are thorough and accurate.

---

[Chelebii]

Addressed the Claude review feedback in b014d43.

What changed:

• clean up the temp file if the EPERM fallback fs.cp path fails
• restore best-effort temp cleanup when the temp-file writeFile step fails
• add regression coverage for both the fs.cp failure path and temp-write cleanup

Validation:

• npm run test --workspace @modelcontextprotocol/server-filesystem -- __tests__/lib.test.ts
• npm run build --workspace @modelcontextprotocol/server-filesystem