fix(filesystem): verify write_file output landed on disk (#4138)#4274
Open
0ywfe wants to merge 1 commit into
Open
fix(filesystem): verify write_file output landed on disk (#4138)#42740ywfe wants to merge 1 commit into
0ywfe wants to merge 1 commit into
Conversation
…tprotocol#4138) write_file in the filesystem server resolved successfully even when no bytes reached disk on Windows. Callers (Claude Desktop and any other MCP client) received "File created successfully" and a normal structured result, with no way to detect that the file was never written. Reported in modelcontextprotocol#4138 against Windows 11 with both isUsingBuiltInNodeForMcp and system Node. Repro ruled out NTFS permissions, Defender, Controlled Folder Access, OneDrive, and paths-with-spaces. The same paths work for edit_file, read_file, and list_directory; only write_file silently no-ops. The host-side root cause varies by environment and isn't fully nailed down, but the failure mode is consistent: fs.writeFile resolves with no error and nothing is on disk afterward. Rather than chase the host layer, this change closes the gap at the boundary we control by adding a post-write verification step to writeFileContent: 1. fs.stat the target file after the write completes. 2. Compare on-disk size to Buffer.byteLength(content, 'utf-8'). 3. Throw an explicit error on miss / mismatch. This converts the silent-success failure mode into a clear "write reported success but the file is missing on disk" error that callers can surface to the user instead of returning a false-positive success. The verification runs for both the fresh-write (wx flag) and atomic- rename (EEXIST) branches. The fix is platform-agnostic: silent success is a bad failure mode on any OS, and the same verification protects against partial writes, antivirus quarantine mid-write, and rename-to-wrong-path bugs in addition to the original Windows symptom.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #4138 —
write_filein the filesystem server silently returned success on Windows even though no bytes reached disk. Callers received"File created successfully"with no way to detect the failure.This change adds a post-write verification step inside
writeFileContent:fs.statthe target after the write completes.Buffer.byteLength(content, 'utf-8').The fix runs for both write paths (
wxfresh-create andEEXISTatomic-rename) and converts the silent failure into a useful error message.Why this approach
The host-side root cause varies by environment (the reporter ruled out NTFS permissions, Defender, Controlled Folder Access, OneDrive, paths-with-spaces — and reproduced on both
isUsingBuiltInNodeForMcp: trueand system Node). What's consistent is the failure mode:fs.writeFileresolves with no error and nothing is on disk.Rather than chase the host layer, this closes the gap at the boundary we control. The verification is platform-agnostic — silent success is the wrong contract on any OS — so the same guard also catches partial writes, antivirus quarantine mid-write, and rename-to-wrong-path bugs.
Test plan
npm test— all 150 existing tests pass.src/filesystem/__tests__/lib.test.ts(`writeFileContent` block):fs.statand assert it is consulted.fs.writeFileto resolve andfs.statto reject withENOENT, asserts a clear"reported success but the file is missing on disk"error.fs.statto report 0 bytes for non-empty content, asserts size-mismatch error.Buffer.byteLength('héllo 🌍', 'utf-8')is the right yardstick.npm run build— TypeScript clean.Out of scope
edit_file/applyFileEdits. The reporter confirmsedit_fileworks correctly on the same paths, so it is not affected by the same failure mode. If desired in a follow-up, the samefs.stat-after-write check could be extracted into a helper.Files
src/filesystem/lib.ts(+22)src/filesystem/__tests__/lib.test.ts(+37, -2)