Repository navigation
Conversation
Since Node 23, Node's `fs.rmSync` on macOS is libc++'s `std::filesystem::remove_all`, which opens every path it removes with `O_DIRECTORY | O_NOFOLLOW` and unlinks the path when that open fails with `ENOTDIR` or `ELOOP`. fspy recorded the failed open as a read, so a task that cleans and rebuilds its output folder had each output that existed beforehand as an input, and missed the cache once a later step changed one. An `O_DIRECTORY` open for reading, without `O_CREAT`, of a path that is there and isn't a folder is no longer recorded, in the preload library and in the Linux seccomp handler. An open of a folder, or of a path that isn't there, is still recorded, since tools such as Go's `os.ReadDir` list folders that way. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
fspy benchmarklinuxmacoswindows |
Drop the fstatat check: an O_DIRECTORY open reads nothing, whether it opens a folder or fails on a file, and listing the folder is recorded on its own. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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 macOS,
vp runrecords the files a task deletes with Node'sfs.rmSyncas files it read. Since Node 23,fs.rmSyncis libc++'sstd::filesystem::remove_all. It opens every path it removes withO_RDONLY | O_DIRECTORY | O_NOFOLLOWto tell files from folders, and unlinks the path when that open fails withENOTDIRorELOOP. fspy classifies every open without write access as a read, so each removed file became an inferred input, fingerprinted as the task left it.So when a build cleans its output folder and writes it again, and a later step changes one of the outputs, for example by stamping a version into it, the next run misses with
'<file>' modified, though the task never read the file. This is the macOS counterpart of #980. We hit it with a runtime build that callsrmSyncon its output folder before rebuilding it, which missed on local macOS runs and hit on Linux CI. On Linux, Node'sfs.rmSyncis libstdc++'s, which tells files from folders by their directory entries and unlinks files without opening them.Before Node 23,
fs.rmSynclstats each path instead. A stat is still recorded as a read on every platform, so this change doesn't affect that case.Changes
O_DIRECTORYopen is no longer recorded, in the preload library on macOS and Linux and in the seccomp handler for static executables on Linux, as delete-only opens on Windows aren't since fix(fspy): don't treat deleting a file as reading it on Windows #980. Such an open reads nothing: it opens only a folder, and reading the folder's entries is recorded as before, throughopendir,fdopendir,scandir,getdirentriesorgetdents64. Both share one classification of the open flags, and the preload'sToAccessModereturnsOption<AccessMode>, as on Windows.O_DIRECTORYopen of a folder that the task then doesn't list no longer makes the folder an input. The accesses under the folder still are. Nor is a missing folder an input when a task lists it with anO_DIRECTORYopen and no stat first, so creating that folder later doesn't make the next run miss, unless a listing of its parent or a stat of it was recorded. With the preload library,opendirandscandirstill record a missing folder, but a bare Goos.ReadDiron macOS doesn't. With the seccomp handler, which tracks static executables and every process on musl,opendiritself opens withO_DIRECTORY, so no listing of a missing folder is recorded there, Node'sreaddiron Alpine included.O_TMPFILEincludesO_DIRECTORY, so that open isn't recorded either. It recorded a write of the folder, which vt ignores, and thelinkatthat gives the file a name is recorded as a write.vtt fs rmnow removes each path the way libc++ does, opening it as a folder first. That way, fix(fspy): don't treat deleting a file as reading it on Windows #980'soutput_changed_after_clean_and_rebuildcase covers this change. Before this change, it failed on macOS and Linux withcache miss: 'out/a.txt' modified. On Windows,vtt fs rmstill usesremove_dir_all, which, like the MSVC STL'sremove_all, deletes each path with a delete-only open, which fix(fspy): don't treat deleting a file as reading it on Windows #980 stopped recording.rust_std::open_directory(preload) andstatic_executable::open_directory(seccomp) check that anO_DIRECTORYopen isn't recorded.🤖 Generated with Claude Code
Note
Medium Risk
Changes core file-access classification used for task cache keys; behavior is narrow and well-tested, but wrong classification would still cause false cache hits or misses.
Overview
Stops treating
O_DIRECTORYopens as input reads in Unix file tracking (preload and seccomp), via sharedopen_access_modeinfspy_shared_unix. When flags includeO_DIRECTORY, tracking records nothing; directory listing still comes fromreaddir/opendirpaths as before.This fixes incorrect cache misses on macOS when tasks delete outputs with Node 23+
fs.rmSync(libc++remove_allprobes each path withO_DIRECTORYfirst). The CHANGELOG fix now covers Windows and macOS together with Linux-aligned behavior.ToAccessMode::to_access_modenow returnsOption<AccessMode>; callers skip IPC when the mode isNone.vtt fs rmon Unix mirrors the same delete semantics (try directory open, then remove file) so integration tests match production. Newopen_directorytests cover preload and static seccomp binaries.Reviewed by Cursor Bugbot for commit 21fa937. Bugbot is set up for automated code reviews on this repo. Configure here.