Repository navigation
fix(fspy): don't treat deleting a file as reading it on Windows - #980
Merged
Merged
Conversation
Deleting a file or folder on Windows opens it with DELETE access and no access to its data. DeleteFileW, RemoveDirectoryW, std's remove_dir_all, libuv's unlink and the MSVC STL's remove, which Node's rmSync uses, all do this, and MoveFileExW opens a rename's source the same way. fspy recorded every open without write access as a read, so the deleted paths became inferred inputs of the task. When the task then wrote such a path again, as a build does after cleaning its output folder, the input was fingerprinted as the task left it. A later step that changed the file, such as one stamping a version into the output, made the next run miss with "'<file>' modified", though the task never read the file. On Unix, unlink, rmdir and a rename's source aren't recorded, so the same task hit there. An open that asks for DELETE and no data access is now not recorded, as on Unix. Recording it as a write instead would make deleted paths outputs and the folders above them written in, which Unix doesn't do. FILE_DELETE_ON_CLOSE requires DELETE, so it needs no check of its own: a temporary file deleted on close is still a read or a write by its data access. Opens that only query attributes, such as a stat, are still reads. CreateSymbolicLinkW creates a file link with such an open, which is no longer recorded either; the reparse point it then sets is still recorded as a write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fspy benchmarklinuxmacoswindows |
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.
Motivation
On Windows,
vp runrecords the files a task deletes as files it read. Deleting a file opens it withDELETEaccess and no data access.DeleteFileW,RemoveDirectoryW, Rust'sremove_dir_all, libuv's unlink, and the MSVC STL'sremove(which Node'srmSyncuses) all do this, and so doesMoveFileExWfor a rename's source. fspy classifies every open without write access as a read.So when a build cleans its output folder and writes it again, every output that existed beforehand becomes an inferred input. The cache fingerprints it as the task left it. If anything then changes that file before the next run, the next run misses with
'<file>' modified, though the task never read it. Examples are a later step stamping a version into the output, or an earlier build in the same job leaving different content. On Linux and macOS,unlink,rmdirand a rename's source aren't recorded, so the same task hits there.We hit this while replaying a monorepo's CI history against the remote cache. One task starts with
rmSync(dist/runtime)and rebuilds the folder, and a later uncached step stamps the commit into its manifest. Across 200 Windows pushes, that task missed on the manifest 101 times, 90 of them in runs where the same task hit on Ubuntu. Each miss cost 9–12 s on the Windows critical path.Changes
DELETEand no data access (FILE_READ_DATA,FILE_WRITE_DATA,FILE_APPEND_DATA,FILE_EXECUTE, anyGENERIC_*right, orMAXIMUM_ALLOWED) is no longer recorded, as on Unix. Recording it as a write instead would make deleted paths outputs, and the folders above them written in, which Unix doesn't do.DELETEtogether with read access is still a read, attribute-only opens such as a stat are still reads, andFILE_DELETE_ON_CLOSEneeds no check of its own because it requiresDELETE.CreateSymbolicLinkWcreates a file link with such an open, so that open is no longer recorded either; the reparse point it then sets is still recorded as a write. The Windowssymlink_destinationtest now checks that a file link is recorded only as a write.output_changed_after_clean_and_rebuild: an earlier build leaves an output, the cached task deletes its output folder and writes it again, an uncached step changes the output, and the next run hits and restores the task's output. It failed on Windows before this change withcache miss: 'out/a.txt' modified, and passed on Linux and macOS.🤖 Generated with Claude Code
Note
Medium Risk
Changes how Windows infers task inputs for caching; misclassification could still cause wrong hits or misses, though behavior is intentionally aligned with Unix and is well-tested.
Overview
Windows file-access tracking no longer records opens that request
DELETEwithout data access, matching Unix behavior whereunlink,rmdir, and rename sources are not tracked. Previously those opens were classified as reads, so tasks that clean an output folder (e.g.rmSync(dist)) incorrectly fingerprinted pre-existing outputs as inputs and could miss the cache when another step later changed those files.The NT hook path now treats
access_mask_to_modeasOption<AccessMode>andhandle_openskips recording when the mask is delete-only (including typicalDeleteFileW/remove_dir_allpatterns). Symbolic link creation is aligned too:CreateSymbolicLinkW's delete-only open is ignored while the reparse-point write is still recorded.Coverage adds unit tests for delete-only masks, tweaks the symlink std test on Windows, a new e2e case (
output_changed_after_clean_and_rebuild) expecting a cache hit after an uncached stamp step, and a changelog entry.Reviewed by Cursor Bugbot for commit e14ae71. Bugbot is set up for automated code reviews on this repo. Configure here.