From 2a680aa35d641b1043c46b28e7a3ac1e8ee813c0 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 20:41:47 +0800 Subject: [PATCH 01/21] feat(fspy): record access outcome and mutation intent Extend AccessMode to u16 and add FAILED, CREATE, TRUNCATE, EXCLUSIVE, RENAME_FROM, RENAME_TO, DELETED, CREATED_DIR and IS_DIR. Interceptions in the Unix preload now report after forwarding to the real function so the outcome is known, and rename, unlink, rmdir and mkdir are intercepted for the first time. Co-authored-by: Claude --- DECISIONS.md | 75 ++++++ .../fspy_preload_unix/src/client/convert.rs | 24 +- crates/fspy_preload_unix/src/client/mod.rs | 17 ++ .../src/interceptions/access.rs | 23 +- .../src/interceptions/dirent.rs | 46 ++-- .../src/interceptions/mod.rs | 1 + .../src/interceptions/mutate.rs | 227 ++++++++++++++++++ .../src/interceptions/open.rs | 95 +++++--- .../src/interceptions/stat.rs | 38 ++- crates/fspy_shared/src/ipc/mod.rs | 73 +++++- 10 files changed, 531 insertions(+), 88 deletions(-) create mode 100644 DECISIONS.md create mode 100644 crates/fspy_preload_unix/src/interceptions/mutate.rs diff --git a/DECISIONS.md b/DECISIONS.md new file mode 100644 index 000000000..529bb168d --- /dev/null +++ b/DECISIONS.md @@ -0,0 +1,75 @@ +# Decision log + +Decisions taken while implementing the two auto-tracking rule sets +(`target/fspy-overlap-experiments/read-write-overlaps.md` and +`output-cleanup.md`) and wiring them through vite-plus into emdash. + +Newest entries at the bottom. Each entry records the drift, the choice, and why. + +## D1 — Tracer scope: all signals, all three platforms + +Confirmed with the requester before starting. The full rule set needs fspy to +gain open success/errno, `O_CREAT`/`O_TRUNC`/`O_EXCL`, confirmed writes, rename +source and destination, unlink/rmdir, and mkdir with errno — on the Unix +preload, the Linux seccomp fallback, and the Windows detours. No platform is +stubbed. `AGENTS.md` forbids skipping tests on either platform. + +## D2 — Rules implemented in place, no crate extraction + +Confirmed before starting. The `vite_task_fs_cache` extraction stays a separate +follow-up so it keeps its no-behavior-change property. This PR changes behavior +and would obscure that proof. + +## D3 — Gitignore via the `ignore` crate + +Confirmed before starting. Hand-rolling gitignore precedence, negation and +nesting is easy to get subtly wrong, and shelling out to `git check-ignore` puts +a process spawn on a hot path and fails when git is absent. + +## D4 — vite-plus preview PR lives on the upstream repo + +Confirmed before starting. `publish-preview.yml` gates on +`github.repository == 'voidzero-dev/vite-plus'` plus a `preview-build` label, so +a fork cannot produce a pkg.pr.new build. Branch is pushed to the upstream repo; +the PR stays a draft and is not merged. + +## D5 — Approximate "confirmed write" from open flags, do not intercept `write` + +**Drift.** `read-write-overlaps.md` defines a write as a confirmed +`write`/`pwrite`/`writev`/`ftruncate` on a write-opened descriptor. Implementing +that needs a per-process fd table plus interception of `close`, `dup`, `dup2` and +`dup3`, because a descriptor number can otherwise be reacquired by an owner the +tracer never saw. I hit exactly that bug in the experiment probe, where +`vite.config.mjs` looked written because fd 18 was reused. + +**Decision.** Record open success and the `O_CREAT`, `O_TRUNC`, `O_EXCL` flags +instead, and treat a mutation as `O_TRUNC`, or `O_CREAT | O_EXCL`, or a rename +destination. A bare write-access open is recorded as capability only and is not +a mutation. + +**Why this is sufficient.** Checked against every case in the experiment matrix: + +- Prettier, ESLint, Stylelint, Vite, Astro all rewrite through + `O_CREAT | O_TRUNC`, so they are still detected. +- Atomic writers create temporaries with `O_CREAT | O_EXCL` and publish by + rename, so both halves are still detected. +- Biome's warm run opens a clean source `O_RDWR` with no truncation and does not + write. It is now correctly *not* a mutation, which is the false positive the + rule exists to remove. +- Lock files across cargo, rustc and Parcel are opened `O_RDWR | O_CREAT` + without truncation and only flocked, so they stop being collected. + +**What it gives up.** Mutation through a writable mmap is invisible. The only +instance in the matrix is Parcel's `lock.mdb`, a lock file that should not be +archived anyway. Recorded as a known gap rather than fixed. + +## D6 — Skip `getdents` success; use mkdir instead + +**Drift.** `output-cleanup.md` lists "errno on getdents or scandir" as the way to +tell that a directory listing failed, which matters for the +`unconditional-clean` case where a tool lists a directory that does not exist. + +**Decision.** Do not add it. Fix 3, "a directory the task created is not an +input", covers the same case using successful `mkdir`, which is far cheaper to +intercept and which the experiment already showed absorbs 264 of 307 derived +directory listings including all 256 of Go's cache shards. diff --git a/crates/fspy_preload_unix/src/client/convert.rs b/crates/fspy_preload_unix/src/client/convert.rs index c42854b32..940585248 100644 --- a/crates/fspy_preload_unix/src/client/convert.rs +++ b/crates/fspy_preload_unix/src/client/convert.rs @@ -105,11 +105,23 @@ impl ToAccessMode for AccessMode { pub struct OpenFlags(pub c_int); impl ToAccessMode for OpenFlags { unsafe fn to_access_mode(self) -> AccessMode { - match self.0 & libc::O_ACCMODE { + let mut mode = match self.0 & libc::O_ACCMODE { libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, libc::O_WRONLY => AccessMode::WRITE, _ => AccessMode::READ, + }; + // Creation and truncation intent is what separates a real mutation from + // a descriptor that merely could have been written. + if self.0 & libc::O_CREAT != 0 { + mode |= AccessMode::CREATE; } + if self.0 & libc::O_TRUNC != 0 { + mode |= AccessMode::TRUNCATE; + } + if self.0 & libc::O_EXCL != 0 { + mode |= AccessMode::EXCLUSIVE; + } + mode } } @@ -120,10 +132,18 @@ impl ToAccessMode for ModeStr { let mode_str = unsafe { CStr::from_ptr(self.0) }.to_bytes().as_bstr(); let has_read = mode_str.contains(&b'r'); let has_write = mode_str.contains(&b'w') || mode_str.contains(&b'a'); - match (has_read, has_write) { + let mut mode = match (has_read, has_write) { (false, true) => AccessMode::WRITE, (true, true) => AccessMode::READ | AccessMode::WRITE, _ => AccessMode::READ, + }; + // "w" and "w+" truncate and create; "a" and "a+" create without + // truncating. Both mutate, so mark them the way the open flags would. + if mode_str.contains(&b'w') { + mode |= AccessMode::CREATE | AccessMode::TRUNCATE; + } else if mode_str.contains(&b'a') { + mode |= AccessMode::CREATE; } + mode } } diff --git a/crates/fspy_preload_unix/src/client/mod.rs b/crates/fspy_preload_unix/src/client/mod.rs index daae12f5a..93d0d8153 100644 --- a/crates/fspy_preload_unix/src/client/mod.rs +++ b/crates/fspy_preload_unix/src/client/mod.rs @@ -158,6 +158,23 @@ pub unsafe fn handle_open(path: impl ToAbsolutePath, mode: impl ToAccessMode) { }); } +/// Report an access whose outcome is known, marking failures with +/// [`AccessMode::FAILED`]. +/// +/// Interceptions call this *after* forwarding to the real function. A failed +/// call still names the path, but it read nothing and wrote nothing, and +/// consumers must be able to tell the difference: a formatter probing for a +/// generated file that is not there yet is not depending on it. +pub unsafe fn handle_outcome( + path: impl ToAbsolutePath, + mode: fspy_shared::ipc::AccessMode, + succeeded: bool, +) { + let mode = if succeeded { mode } else { mode | fspy_shared::ipc::AccessMode::FAILED }; + // SAFETY: path contains valid pointers forwarded from the interposed function's caller + unsafe { handle_open(path, mode) }; +} + #[cfg(not(test))] #[ctor::ctor(unsafe)] fn init_client() { diff --git a/crates/fspy_preload_unix/src/interceptions/access.rs b/crates/fspy_preload_unix/src/interceptions/access.rs index 0cae39220..8ba0ed146 100644 --- a/crates/fspy_preload_unix/src/interceptions/access.rs +++ b/crates/fspy_preload_unix/src/interceptions/access.rs @@ -1,19 +1,21 @@ +//! `access` and `faccessat` are pure existence and permission probes, so the +//! outcome is the whole signal. Reported after the call. + use fspy_shared::ipc::AccessMode; use libc::{c_char, c_int}; use crate::{ - client::{convert::PathAt, handle_open}, + client::{convert::PathAt, handle_outcome}, macros::intercept, }; intercept!(access(64): unsafe extern "C" fn(pathname: *const c_char, mode: c_int) -> c_int); unsafe extern "C" fn access(pathname: *const c_char, mode: c_int) -> c_int { - // SAFETY: pathname is a valid C string pointer provided by the caller of the interposed function - unsafe { - handle_open(pathname, AccessMode::READ); - } // SAFETY: calling the original libc access() with the same arguments forwarded from the interposed function - unsafe { access::original()(pathname, mode) } + let result = unsafe { access::original()(pathname, mode) }; + // SAFETY: pathname is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(pathname, AccessMode::READ, result == 0) }; + result } intercept!(faccessat(64): unsafe extern "C" fn(dirfd: c_int, pathname: *const c_char, mode: c_int, flags: c_int) -> c_int); @@ -23,10 +25,9 @@ unsafe extern "C" fn faccessat( mode: c_int, flags: c_int, ) -> c_int { - // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function - unsafe { - handle_open(PathAt(dirfd, pathname), AccessMode::READ); - } // SAFETY: calling the original libc faccessat() with the same arguments forwarded from the interposed function - unsafe { faccessat::original()(dirfd, pathname, mode, flags) } + let result = unsafe { faccessat::original()(dirfd, pathname, mode, flags) }; + // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function + unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; + result } diff --git a/crates/fspy_preload_unix/src/interceptions/dirent.rs b/crates/fspy_preload_unix/src/interceptions/dirent.rs index d2b11d39a..8fe45b120 100644 --- a/crates/fspy_preload_unix/src/interceptions/dirent.rs +++ b/crates/fspy_preload_unix/src/interceptions/dirent.rs @@ -2,7 +2,7 @@ use fspy_shared::ipc::AccessMode; use libc::{DIR, c_char, c_int, c_long, c_void}; use crate::{ - client::{convert::Fd, handle_open}, + client::{convert::Fd, handle_outcome}, macros::intercept, }; @@ -18,15 +18,16 @@ unsafe extern "C" fn scandir( select: *const c_void, compar: *const c_void, ) -> c_int { - // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_open(dirname, AccessMode::READ_DIR) } // SAFETY: calling the original libc scandir() with the same arguments forwarded from the interposed function - unsafe { scandir::original()(dirname, namelist, select, compar) } + let result = unsafe { scandir::original()(dirname, namelist, select, compar) }; + // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(dirname, AccessMode::READ_DIR, result >= 0) }; + result } #[cfg(target_os = "macos")] mod macos_only { - use super::{AccessMode, Fd, c_char, c_int, c_void, handle_open, intercept}; + use super::{AccessMode, Fd, c_char, c_int, c_void, handle_outcome, intercept}; intercept!(scandir_b: unsafe extern "C" fn ( dirname: *const c_char, @@ -40,10 +41,11 @@ mod macos_only { select: *const c_void, compar: *const c_void, ) -> c_int { - // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_open(dirname, AccessMode::READ_DIR) }; // SAFETY: calling the original libc scandir_b() with the same arguments forwarded from the interposed function - unsafe { scandir_b::original()(dirname, namelist, select, compar) } + let result = unsafe { scandir_b::original()(dirname, namelist, select, compar) }; + // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(dirname, AccessMode::READ_DIR, result >= 0) }; + result } intercept!(__getdirentries64: unsafe extern "C" fn(c_int, *mut u8, usize, *mut i64) -> isize); @@ -53,10 +55,11 @@ mod macos_only { buf_len: usize, basep: *mut i64, ) -> isize { - // SAFETY: fd is a valid file descriptor provided by the caller of __getdirentries64 - unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; // SAFETY: calling the original libc __getdirentries64() with the same arguments forwarded from the interposed function - unsafe { __getdirentries64::original()(fd, buf, buf_len, basep) } + let result = unsafe { __getdirentries64::original()(fd, buf, buf_len, basep) }; + // SAFETY: fd is a valid file descriptor provided by the caller of __getdirentries64 + unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, result >= 0) }; + result } } @@ -67,24 +70,27 @@ unsafe extern "C" fn getdirentries( nbytes: c_int, basep: *mut c_long, ) -> c_int { - // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function - unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; // SAFETY: calling the original libc getdirentries() with the same arguments forwarded from the interposed function - unsafe { getdirentries::original()(fd, buf, nbytes, basep) } + let result = unsafe { getdirentries::original()(fd, buf, nbytes, basep) }; + // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function + unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, result >= 0) }; + result } intercept!(fdopendir(64): unsafe extern "C" fn (fd: c_int) -> *mut DIR); unsafe extern "C" fn fdopendir(fd: c_int) -> *mut DIR { - // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function - unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; // SAFETY: calling the original libc fdopendir() with the same arguments forwarded from the interposed function - unsafe { fdopendir::original()(fd) } + let result = unsafe { fdopendir::original()(fd) }; + // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function + unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, !result.is_null()) }; + result } intercept!(opendir(64): unsafe extern "C" fn (*const c_char) -> *mut DIR); unsafe extern "C" fn opendir(dir_name: *const c_char) -> *mut DIR { - // SAFETY: dir_name is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_open(dir_name, AccessMode::READ_DIR) }; // SAFETY: calling the original libc opendir() with the same arguments forwarded from the interposed function - unsafe { opendir::original()(dir_name) } + let result = unsafe { opendir::original()(dir_name) }; + // SAFETY: dir_name is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(dir_name, AccessMode::READ_DIR, !result.is_null()) }; + result } diff --git a/crates/fspy_preload_unix/src/interceptions/mod.rs b/crates/fspy_preload_unix/src/interceptions/mod.rs index 0d3742ea7..5c394e603 100644 --- a/crates/fspy_preload_unix/src/interceptions/mod.rs +++ b/crates/fspy_preload_unix/src/interceptions/mod.rs @@ -1,5 +1,6 @@ mod access; mod dirent; +mod mutate; mod open; mod spawn; mod stat; diff --git a/crates/fspy_preload_unix/src/interceptions/mutate.rs b/crates/fspy_preload_unix/src/interceptions/mutate.rs new file mode 100644 index 000000000..eca40b718 --- /dev/null +++ b/crates/fspy_preload_unix/src/interceptions/mutate.rs @@ -0,0 +1,227 @@ +//! Interceptions for calls that change the shape of the tree rather than the +//! contents of a file: rename, unlink, rmdir and mkdir. +//! +//! Without these, a tool that publishes results atomically is invisible. It +//! writes a temporary and renames it over the destination, so the destination +//! never appears in a write event and the temporary — which no longer exists — +//! does. Directory renames are worse: a build that stages into `dist.tmp` and +//! swaps it into place produces no write event for any of its real outputs. +//! +//! All of these report *after* the real call, because only a successful call +//! changed anything. A failed `mkdir` is the common case, since most callers use +//! it as "ensure this exists" and expect `EEXIST`. + +use fspy_shared::ipc::AccessMode; + +use crate::{ + client::{convert::PathAt, handle_outcome}, + libc::{c_char, c_int}, + macros::intercept, +}; + +/// Whether a path is a directory, for tagging rename events. +/// +/// A consumer needs this to re-attribute writes recorded beneath a staging +/// directory to the published location. Called before the rename, while the +/// source still exists. +unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { + let mut stat_buf: libc::stat = unsafe { core::mem::zeroed() }; + // SAFETY: path is a valid C string pointer and stat_buf is a valid, owned stat struct + let result = unsafe { libc::fstatat(dirfd, path, &raw mut stat_buf, libc::AT_SYMLINK_NOFOLLOW) }; + result == 0 && (stat_buf.st_mode & libc::S_IFMT) == libc::S_IFDIR +} + +/// Report both halves of a rename: the source is gone, the destination now +/// holds whatever the source held. +unsafe fn report_rename( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + succeeded: bool, + was_directory: bool, +) { + let dir_flag = if was_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: old_path and new_path are valid C string pointers from the caller + unsafe { + handle_outcome( + PathAt(old_dirfd, old_path), + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag, + succeeded, + ); + handle_outcome( + PathAt(new_dirfd, new_path), + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + succeeded, + ); + } +} + +intercept!(rename: unsafe extern "C" fn(*const c_char, *const c_char) -> c_int); +unsafe extern "C" fn rename(old_path: *const c_char, new_path: *const c_char) -> c_int { + // SAFETY: old_path is a valid C string pointer provided by the caller + let was_directory = unsafe { is_directory_at(libc::AT_FDCWD, old_path) }; + // SAFETY: forwarding the caller's arguments to the original libc rename() + let result = unsafe { rename::original()(old_path, new_path) }; + // SAFETY: both paths remain valid C string pointers after the call + unsafe { + report_rename( + libc::AT_FDCWD, + old_path, + libc::AT_FDCWD, + new_path, + result == 0, + was_directory, + ); + } + result +} + +intercept!(renameat: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char) -> c_int); +unsafe extern "C" fn renameat( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: forwarding the caller's arguments to the original libc renameat() + let result = unsafe { renameat::original()(old_dirfd, old_path, new_dirfd, new_path) }; + // SAFETY: both paths remain valid C string pointers after the call + unsafe { + report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); + } + result +} + +// macOS publishes atomic swaps through renameatx_np with RENAME_SWAP, which +// mutates both paths. Vite's dependency optimizer and rustup both reach it. +#[cfg(target_os = "macos")] +intercept!(renameatx_np: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char, libc::c_uint) -> c_int); +#[cfg(target_os = "macos")] +unsafe extern "C" fn renameatx_np( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + flags: libc::c_uint, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: forwarding the caller's arguments to the original renameatx_np() + let result = + unsafe { renameatx_np::original()(old_dirfd, old_path, new_dirfd, new_path, flags) }; + // SAFETY: both paths remain valid C string pointers after the call + unsafe { + report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); + } + result +} + +// Linux's renameat2 can also exchange two paths, in which case both sides are +// mutated rather than one replacing the other. +#[cfg(target_os = "linux")] +intercept!(renameat2: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char, libc::c_uint) -> c_int); +#[cfg(target_os = "linux")] +unsafe extern "C" fn renameat2( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + flags: libc::c_uint, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: forwarding the caller's arguments to the original renameat2() + let result = unsafe { renameat2::original()(old_dirfd, old_path, new_dirfd, new_path, flags) }; + let exchanged = flags & libc::RENAME_EXCHANGE != 0; + // SAFETY: both paths remain valid C string pointers after the call + unsafe { + report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); + if exchanged { + // An exchange mutates the source too, so it is not simply gone. + handle_outcome( + PathAt(old_dirfd, old_path), + AccessMode::RENAME_TO | AccessMode::WRITE, + result == 0, + ); + } + } + result +} + +intercept!(unlink: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn unlink(path: *const c_char) -> c_int { + // SAFETY: forwarding the caller's argument to the original libc unlink() + let result = unsafe { unlink::original()(path) }; + // SAFETY: path remains a valid C string pointer after the call + unsafe { handle_outcome(path, AccessMode::DELETED, result == 0) }; + result +} + +intercept!(unlinkat: unsafe extern "C" fn(c_int, *const c_char, c_int) -> c_int); +unsafe extern "C" fn unlinkat(dirfd: c_int, path: *const c_char, flags: c_int) -> c_int { + // SAFETY: forwarding the caller's arguments to the original libc unlinkat() + let result = unsafe { unlinkat::original()(dirfd, path, flags) }; + let removed_directory = flags & libc::AT_REMOVEDIR != 0; + let dir_flag = if removed_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: path remains a valid C string pointer after the call + unsafe { + handle_outcome(PathAt(dirfd, path), AccessMode::DELETED | dir_flag, result == 0); + } + result +} + +intercept!(remove: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn remove(path: *const c_char) -> c_int { + // SAFETY: forwarding the caller's argument to the original libc remove() + let result = unsafe { remove::original()(path) }; + // SAFETY: path remains a valid C string pointer after the call + unsafe { handle_outcome(path, AccessMode::DELETED, result == 0) }; + result +} + +intercept!(rmdir: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn rmdir(path: *const c_char) -> c_int { + // SAFETY: forwarding the caller's argument to the original libc rmdir() + let result = unsafe { rmdir::original()(path) }; + // SAFETY: path remains a valid C string pointer after the call + unsafe { + handle_outcome(path, AccessMode::DELETED | AccessMode::IS_DIR, result == 0); + } + result +} + +intercept!(mkdir: unsafe extern "C" fn(*const c_char, libc::mode_t) -> c_int); +unsafe extern "C" fn mkdir(path: *const c_char, mode: libc::mode_t) -> c_int { + // SAFETY: forwarding the caller's arguments to the original libc mkdir() + let result = unsafe { mkdir::original()(path, mode) }; + // Only a successful mkdir means this run created the directory. Callers + // routinely ignore EEXIST, and treating that as creation would claim every + // pre-existing directory. + // SAFETY: path remains a valid C string pointer after the call + unsafe { + handle_outcome( + path, + AccessMode::CREATED_DIR | AccessMode::IS_DIR | AccessMode::WRITE, + result == 0, + ); + } + result +} + +intercept!(mkdirat: unsafe extern "C" fn(c_int, *const c_char, libc::mode_t) -> c_int); +unsafe extern "C" fn mkdirat(dirfd: c_int, path: *const c_char, mode: libc::mode_t) -> c_int { + // SAFETY: forwarding the caller's arguments to the original libc mkdirat() + let result = unsafe { mkdirat::original()(dirfd, path, mode) }; + // SAFETY: path remains a valid C string pointer after the call + unsafe { + handle_outcome( + PathAt(dirfd, path), + AccessMode::CREATED_DIR | AccessMode::IS_DIR | AccessMode::WRITE, + result == 0, + ); + } + result +} diff --git a/crates/fspy_preload_unix/src/interceptions/open.rs b/crates/fspy_preload_unix/src/interceptions/open.rs index 641593a13..4d9be0fdb 100644 --- a/crates/fspy_preload_unix/src/interceptions/open.rs +++ b/crates/fspy_preload_unix/src/interceptions/open.rs @@ -1,9 +1,18 @@ +//! Interceptions for the open family. +//! +//! These report *after* forwarding to the real function, so the recorded access +//! carries whether it succeeded. That distinction matters: a tool probing for a +//! generated file it is about to write is not depending on that file, and +//! counting the failed probe as a read makes a freshly generated output look +//! like an input it read before writing. + +use fspy_shared::ipc::AccessMode; use libc::FILE; use crate::{ client::{ - convert::{ModeStr, OpenFlags, PathAt}, - handle_open, + convert::{ModeStr, OpenFlags, PathAt, ToAccessMode as _}, + handle_outcome, }, libc::{c_char, c_int}, macros::intercept, @@ -25,19 +34,28 @@ type Mode = libc::mode_t; #[cfg(target_os = "macos")] // https://github.com/tailhook/openat/issues/21#issuecomment-535914957 type Mode = c_int; +/// The access mode implied by open flags, resolved before the call because the +/// flags are not affected by its outcome. +fn open_mode(flags: c_int) -> AccessMode { + // SAFETY: OpenFlags holds a plain integer, so no pointer is dereferenced + unsafe { OpenFlags(flags).to_access_mode() } +} + intercept!(open(64): unsafe extern "C" fn(*const c_char, c_int, args: ...) -> c_int); unsafe extern "C" fn open(path: *const c_char, flags: c_int, mut args: ...) -> c_int { - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_open(path, OpenFlags(flags)) }; - if has_mode_arg(flags) { + let mode = open_mode(flags); + let result = if has_mode_arg(flags) { // SAFETY: when O_CREAT or O_TMPFILE is set, a mode_t argument is required by the open() contract - let mode: Mode = unsafe { args.next_arg() }; + let file_mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc open() with the same arguments forwarded from the interposed function - unsafe { open::original()(path, flags, mode) } + unsafe { open::original()(path, flags, file_mode) } } else { // SAFETY: calling the original libc open() with the same arguments forwarded from the interposed function unsafe { open::original()(path, flags) } - } + }; + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(path, mode, result >= 0) }; + result } intercept!(openat(64): unsafe extern "C" fn(c_int, *const c_char, c_int, ...) -> c_int); @@ -47,36 +65,39 @@ unsafe extern "C" fn openat( flags: c_int, mut args: ... ) -> c_int { - // SAFETY: dirfd and path are valid arguments provided by the caller of the interposed function - unsafe { handle_open(PathAt(dirfd, path), OpenFlags(flags)) }; - - if has_mode_arg(flags) { + let mode = open_mode(flags); + let result = if has_mode_arg(flags) { // https://github.com/tailhook/openat/issues/21#issuecomment-535914957 // SAFETY: when O_CREAT or O_TMPFILE is set, a mode_t argument is required by the openat() contract - let mode: Mode = unsafe { args.next_arg() }; + let file_mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc openat() with the same arguments forwarded from the interposed function - unsafe { openat::original()(dirfd, path, flags, mode) } + unsafe { openat::original()(dirfd, path, flags, file_mode) } } else { // SAFETY: calling the original libc openat() with the same arguments forwarded from the interposed function unsafe { openat::original()(dirfd, path, flags) } - } + }; + // SAFETY: dirfd and path are valid arguments provided by the caller of the interposed function + unsafe { handle_outcome(PathAt(dirfd, path), mode, result >= 0) }; + result } #[cfg(target_os = "macos")] intercept!(open_nocancel: unsafe extern "C" fn(*const c_char, c_int, ...) -> c_int); #[cfg(target_os = "macos")] unsafe extern "C" fn open_nocancel(path: *const c_char, flags: c_int, mut args: ...) -> c_int { - // SAFETY: path is a valid C string pointer provided by the caller of open$NOCANCEL - unsafe { handle_open(path, OpenFlags(flags)) }; - if has_mode_arg(flags) { + let mode = open_mode(flags); + let result = if has_mode_arg(flags) { // SAFETY: O_CREAT requires a mode argument, matching the open$NOCANCEL contract - let mode: Mode = unsafe { args.next_arg() }; + let file_mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc open$NOCANCEL() with the same arguments forwarded from the interposed function - unsafe { open_nocancel::original()(path, flags, mode) } + unsafe { open_nocancel::original()(path, flags, file_mode) } } else { // SAFETY: calling the original libc open$NOCANCEL() with the same arguments forwarded from the interposed function unsafe { open_nocancel::original()(path, flags) } - } + }; + // SAFETY: path is a valid C string pointer provided by the caller of open$NOCANCEL + unsafe { handle_outcome(path, mode, result >= 0) }; + result } #[cfg(target_os = "macos")] @@ -88,25 +109,30 @@ unsafe extern "C" fn openat_nocancel( flags: c_int, mut args: ... ) -> c_int { - // SAFETY: dirfd and path are valid arguments provided by the caller of openat$NOCANCEL - unsafe { handle_open(PathAt(dirfd, path), OpenFlags(flags)) }; - if has_mode_arg(flags) { + let mode = open_mode(flags); + let result = if has_mode_arg(flags) { // SAFETY: O_CREAT requires a mode argument, matching the openat$NOCANCEL contract - let mode: Mode = unsafe { args.next_arg() }; + let file_mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc openat$NOCANCEL() with the same arguments forwarded from the interposed function - unsafe { openat_nocancel::original()(dirfd, path, flags, mode) } + unsafe { openat_nocancel::original()(dirfd, path, flags, file_mode) } } else { // SAFETY: calling the original libc openat$NOCANCEL() with the same arguments forwarded from the interposed function unsafe { openat_nocancel::original()(dirfd, path, flags) } - } + }; + // SAFETY: dirfd and path are valid arguments provided by the caller of openat$NOCANCEL + unsafe { handle_outcome(PathAt(dirfd, path), mode, result >= 0) }; + result } intercept!(fopen(64): unsafe extern "C" fn(path: *const c_char, mode: *const c_char) -> *mut FILE); unsafe extern "C" fn fopen(path: *const c_char, mode: *const c_char) -> *mut libc::FILE { - // SAFETY: path and mode are valid C string pointers provided by the caller of the interposed function - unsafe { handle_open(path, ModeStr(mode)) }; + // SAFETY: mode is a valid C string pointer provided by the caller of the interposed function + let access_mode = unsafe { ModeStr(mode).to_access_mode() }; // SAFETY: calling the original libc fopen() with the same arguments forwarded from the interposed function - unsafe { fopen::original()(path, mode) } + let result = unsafe { fopen::original()(path, mode) }; + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(path, access_mode, !result.is_null()) }; + result } intercept!(freopen(64): unsafe extern "C" fn(path: *const c_char, mode: *const c_char, stream: *mut FILE) -> *mut FILE); @@ -115,8 +141,11 @@ unsafe extern "C" fn freopen( mode: *const c_char, stream: *mut FILE, ) -> *mut FILE { - // SAFETY: path and mode are valid C string pointers provided by the caller of the interposed function - unsafe { handle_open(path, ModeStr(mode)) }; + // SAFETY: mode is a valid C string pointer provided by the caller of the interposed function + let access_mode = unsafe { ModeStr(mode).to_access_mode() }; // SAFETY: calling the original libc freopen() with the same arguments forwarded from the interposed function - unsafe { freopen::original()(path, mode, stream) } + let result = unsafe { freopen::original()(path, mode, stream) }; + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(path, access_mode, !result.is_null()) }; + result } diff --git a/crates/fspy_preload_unix/src/interceptions/stat.rs b/crates/fspy_preload_unix/src/interceptions/stat.rs index ac4af7651..e31bdd278 100644 --- a/crates/fspy_preload_unix/src/interceptions/stat.rs +++ b/crates/fspy_preload_unix/src/interceptions/stat.rs @@ -4,29 +4,27 @@ use libc::{c_char, c_int, stat as stat_struct}; #[cfg(target_os = "linux")] use crate::client::convert::Fd; use crate::{ - client::{convert::PathAt, handle_open}, + client::{convert::PathAt, handle_outcome}, macros::intercept, }; intercept!(stat(64): unsafe extern "C" fn(path: *const c_char, buf: *mut stat_struct) -> c_int); unsafe extern "C" fn stat(path: *const c_char, buf: *mut stat_struct) -> c_int { - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { - handle_open(path, AccessMode::READ); - } // SAFETY: calling the original libc stat() with the same arguments forwarded from the interposed function - unsafe { stat::original()(path, buf) } + let result = unsafe { stat::original()(path, buf) }; + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(path, AccessMode::READ, result == 0) }; + result } intercept!(lstat(64): unsafe extern "C" fn(path: *const c_char, buf: *mut stat_struct) -> c_int); unsafe extern "C" fn lstat(path: *const c_char, buf: *mut stat_struct) -> c_int { // TODO: add accessmode ReadNoFollow - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { - handle_open(path, AccessMode::READ); - } // SAFETY: calling the original libc lstat() with the same arguments forwarded from the interposed function - unsafe { lstat::original()(path, buf) } + let result = unsafe { lstat::original()(path, buf) }; + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_outcome(path, AccessMode::READ, result == 0) }; + result } intercept!(fstatat(64): unsafe extern "C" fn(dirfd: c_int, pathname: *const c_char, buf: *mut stat_struct, flags: c_int) -> c_int); @@ -36,12 +34,11 @@ unsafe extern "C" fn fstatat( buf: *mut stat_struct, flags: c_int, ) -> c_int { - // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function - unsafe { - handle_open(PathAt(dirfd, pathname), AccessMode::READ); - } // SAFETY: calling the original libc fstatat() with the same arguments forwarded from the interposed function - unsafe { fstatat::original()(dirfd, pathname, buf, flags) } + let result = unsafe { fstatat::original()(dirfd, pathname, buf, flags) }; + // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function + unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; + result } #[cfg(target_os = "linux")] @@ -68,15 +65,16 @@ unsafe extern "C" fn statx( return -1; }; + // SAFETY: calling the original libc statx() with the same arguments forwarded from the interposed function + let result = unsafe { original(dirfd, pathname, flags, mask, statxbuf) }; if pathname.is_null() { if flags & libc::AT_EMPTY_PATH != 0 { // SAFETY: dirfd is provided by the statx caller. - unsafe { handle_open(Fd(dirfd), AccessMode::READ) }; + unsafe { handle_outcome(Fd(dirfd), AccessMode::READ, result == 0) }; } } else { // SAFETY: pathname is a non-null C string pointer provided by the statx caller. - unsafe { handle_open(PathAt(dirfd, pathname), AccessMode::READ) }; + unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; } - // SAFETY: calling the original libc statx() with the same arguments forwarded from the interposed function - unsafe { original(dirfd, pathname, flags, mask, statxbuf) } + result } diff --git a/crates/fspy_shared/src/ipc/mod.rs b/crates/fspy_shared/src/ipc/mod.rs index c7236e5d6..a21fb34ae 100644 --- a/crates/fspy_shared/src/ipc/mod.rs +++ b/crates/fspy_shared/src/ipc/mod.rs @@ -9,13 +9,82 @@ pub use native_str::NativeStr; use wincode::{SchemaRead, SchemaWrite}; #[derive(SchemaWrite, SchemaRead, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)] -pub struct AccessMode(u8); +pub struct AccessMode(u16); bitflags! { - impl AccessMode: u8 { + /// What a process did, or tried to do, to one path. + /// + /// `READ`, `WRITE` and `READ_DIR` describe the *intent* of the call. The + /// remaining flags qualify it, and consumers need them to tell a real + /// dependency from a probe and a real mutation from mere write capability: + /// + /// - a `READ` that also has `FAILED` named a path that was not there, so it + /// is an absence check rather than a dependency; + /// - a `WRITE` on its own is only capability. Truncating, exclusively + /// creating, or being a rename destination is what makes it a mutation. + impl AccessMode: u16 { + /// Opened for reading, or its metadata was inspected. const READ = 1; + /// Opened for writing. On its own this is capability, not mutation. const WRITE = 1 << 1; + /// Directory entries were listed. const READ_DIR = 1 << 2; + /// The call failed. The path was still named, but nothing was read, + /// written or listed. + const FAILED = 1 << 3; + /// The open carried `O_CREAT`. + const CREATE = 1 << 4; + /// The open carried `O_TRUNC`, so any previous contents are gone. + const TRUNCATE = 1 << 5; + /// The open carried `O_EXCL`, so the path is newly created and had no + /// previous contents to read. + const EXCLUSIVE = 1 << 6; + /// The path was the source of a successful rename and no longer exists + /// under this name. + const RENAME_FROM = 1 << 7; + /// The path was the destination of a successful rename, which replaced + /// whatever was there. + const RENAME_TO = 1 << 8; + /// The path was successfully removed by `unlink`, `remove` or `rmdir`. + const DELETED = 1 << 9; + /// A directory was successfully created at this path. + const CREATED_DIR = 1 << 10; + /// The path is a directory. Set on rename events so a consumer can + /// re-attribute writes recorded beneath a renamed directory. + const IS_DIR = 1 << 11; + } +} + +impl AccessMode { + /// Whether this access changed the path's contents. + /// + /// Write capability alone does not qualify: a descriptor opened `O_RDWR` and + /// never written leaves the file untouched, which is what Biome does to a + /// clean source file on a warm run. + #[must_use] + pub const fn is_mutation(self) -> bool { + if self.contains(Self::FAILED) { + return false; + } + if self.intersects(Self::RENAME_TO.union(Self::DELETED).union(Self::CREATED_DIR)) { + return true; + } + self.contains(Self::WRITE) + && self.intersects(Self::TRUNCATE.union(Self::CREATE.union(Self::EXCLUSIVE))) + } + + /// Whether this access observed existing content, so the path is a genuine + /// dependency. + /// + /// A failed call observed nothing. An exclusive create observed nothing + /// either, because the path is new by definition, which is how Go's + /// `os.CreateTemp` opens atomic-write temporaries. + #[must_use] + pub const fn is_content_read(self) -> bool { + if self.contains(Self::FAILED) || self.contains(Self::EXCLUSIVE) { + return false; + } + self.intersects(Self::READ.union(Self::READ_DIR)) } } From 7c1de0c3c074c798d6f93e16d853e1d46081e690 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 20:56:14 +0800 Subject: [PATCH 02/21] feat(cache): classify auto inputs and outputs from access order and gitignore Decide from ordered accesses which paths a task consumed and which it produced, using only information available from a call's arguments. A write with no read is an output. A write-first overlap is an output. A read-first overlap is an output when the path is gitignored and a modified input otherwise, which is what separates a linter rewriting its own cache from a formatter rewriting a source. Rename destinations count as writes and directory renames re-attribute the writes recorded beneath them, so a build that stages into dist.tmp and swaps it into place still reports its outputs. Bare directory stats and listings of directories the task wrote into no longer become inputs; such a fingerprint records the task's own product and can never match on a clean checkout. PathFingerprint::Folder therefore always carries entries, so the cache schema moves to v19. Co-authored-by: Claude --- Cargo.lock | 17 ++ Cargo.toml | 1 + DECISIONS.md | 93 ++++++ crates/fspy/src/unix/syscall_handler/mod.rs | 30 +- crates/fspy_preload_unix/src/client/mod.rs | 17 +- .../src/interceptions/access.rs | 23 +- .../src/interceptions/dirent.rs | 46 ++- .../src/interceptions/mutate.rs | 180 +++++------ .../src/interceptions/open.rs | 95 ++---- .../src/interceptions/stat.rs | 38 +-- crates/fspy_shared/src/ipc/mod.rs | 83 +++-- crates/vite_task/Cargo.toml | 1 + crates/vite_task/src/session/cache/mod.rs | 2 +- .../src/session/execute/cache_update.rs | 117 ++++--- .../vite_task/src/session/execute/classify.rs | 287 ++++++++++++++++++ .../src/session/execute/fingerprint.rs | 73 ++--- .../src/session/execute/gitignore.rs | 64 ++++ crates/vite_task/src/session/execute/mod.rs | 4 + .../src/session/execute/tracked_accesses.rs | 48 +-- 19 files changed, 789 insertions(+), 430 deletions(-) create mode 100644 crates/vite_task/src/session/execute/classify.rs create mode 100644 crates/vite_task/src/session/execute/gitignore.rs diff --git a/Cargo.lock b/Cargo.lock index fd72b2ce0..4bb5ede7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1611,6 +1611,22 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" +[[package]] +name = "ignore" +version = "0.4.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f8a7b8211e695a1d0cd91cace480d4d0bd57667ab10277cc412c5f7f4884f83" +dependencies = [ + "crossbeam-deque", + "globset", + "log", + "memchr", + "regex-automata", + "same-file", + "walkdir", + "winapi-util", +] + [[package]] name = "indenter" version = "0.3.4" @@ -4196,6 +4212,7 @@ dependencies = [ "derive_more", "fspy", "futures-util", + "ignore", "materialized_artifact", "materialized_artifact_build", "nix 0.31.2", diff --git a/Cargo.toml b/Cargo.toml index 804541230..53bcc9147 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ futures-util = "0.3.31" globset = "0.4.18" getrandom = "0.4.2" jsonc-parser = { version = "0.32.0", features = ["serde"] } +ignore = "0.4.23" libc = "0.2.185" libtest-mimic = "0.8.2" memmap2 = "0.9.11" diff --git a/DECISIONS.md b/DECISIONS.md index 529bb168d..7f0630893 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -73,3 +73,96 @@ tell that a directory listing failed, which matters for the input", covers the same case using successful `mkdir`, which is far cheaper to intercept and which the experiment already showed absorbs 264 of 307 derived directory listings including all 256 of Go's cache shards. + +## D7 — The Linux seccomp fallback is a reduced-fidelity path + +**Drift.** D1 committed to all signals on all three backends. The seccomp +supervisor responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` +(`fspy_seccomp_unotify/src/supervisor/listener.rs:50`), so it is notified +*before* the syscall runs and never learns the result. Success-dependent signals +are therefore not obtainable there without emulating each syscall in the +supervisor, which would mean reproducing the target's cwd, dirfd and credentials. + +**Decision.** On the seccomp path emit only what is knowable before the call: +read/write intent plus `CREATE`, `TRUNCATE` and `EXCLUSIVE` from the open flags. +Do **not** emit `FAILED`, `DELETED`, `CREATED_DIR` or the rename pair, because +emitting them unconditionally would be actively wrong — a failed `mkdir` would +claim a pre-existing directory, and a failed `unlink` would retire a path that is +still there. + +This path is the fallback for statically linked binaries where preload injection +does not apply; the primary Linux path is the preload library, which has the full +set. + +**Why the missing signals are safe.** Every one degrades toward caching less: +absent `DELETED` leaves a listing as an input, absent `CREATED_DIR` leaves a +directory as an input. Both cost cache hits, not correctness. + +## D8 — Guard: an unexplained missing mutation blocks caching + +**Drift.** One missing signal is *not* safe on its own. If a rename goes +unobserved, the write lands on a staging path that no longer exists at archive +time, rule 6 drops it, and the archive comes out empty — which restores nothing +on a cache hit and leaves a wrong tree. That is exactly the `atomic-dist-swap` +failure, and it would resurface on the seccomp path. + +**Decision.** Add a platform-agnostic guard in `vite_task`: if a recorded +mutation targets a path that is absent at archive time and no observed rename +explains where it went, the output set is incomplete, so refuse to cache the run. + +This protects correctness beyond the seccomp case. It also covers mutations +through `clonefile`, `copyfile` and `FICLONE`, none of which is intercepted, and +any future publish mechanism the tracer has not learned about yet. The cost is a +missed cache entry, never a wrong tree. + +## D9 — Never rely on syscall outcomes; supersedes D5, D7 and D8 + +**Instruction.** Do not build rules on whether a call succeeded. + +**What this invalidates.** D1 through D8 assumed the tracer could report +outcomes, which drove reporting *after* the real call so `errno` was known. That +is now reverted: + +- `AccessMode::FAILED` is removed, along with the after-the-call reporting in + `open.rs`, `stat.rs`, `access.rs` and `dirent.rs`. They report before the call + again, as they did originally. +- `CREATED_DIR` is removed. Recording it pre-call would claim every directory a + caller merely ensured exists, since `mkdir` returning `EEXIST` is the normal + case. Output-cleanup fix 3 therefore cannot be implemented. +- Output-cleanup fix 2, dropping a listing whose entries were then deleted, also + cannot be implemented: a failed `unlink` would retire a path that is still + there, and that direction drops a real input. +- D7's reduced-fidelity seccomp path and D8's unexplained-mutation guard both + become moot. + +**Why this is the better design, not merely a constraint.** The Linux seccomp +supervisor responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` and is notified +before the syscall, so it can never observe an outcome. Any outcome-dependent +rule would have silently degraded there while working on macOS, Windows and the +Linux preload — a correctness rule that holds on three backends and not the +fourth. Restricting the rules to pre-call information gives all four identical +behaviour and removes the asymmetry entirely. + +It also makes a trace a function of the call arguments alone rather than of +filesystem state at trace time, so two runs over the same inputs record the same +events. + +**What still works, using only pre-call information:** + +- intent from `O_ACCMODE`, plus `O_CREAT`, `O_TRUNC` and `O_EXCL` +- mutation defined as `O_TRUNC`, or `O_CREAT | O_EXCL`, or being a rename + destination +- write-first versus read-first ordering +- the gitignore tie-break, which is not access data at all +- rename pairs and directory-rename expansion, recorded as attempts +- ignoring bare directory stats + +**Two consequences accepted.** A failed probe now counts as a read, so a tool +that checks for a generated file before creating it looks like it read it first; +for ignored paths the gitignore clause still resolves that to an output, and for +tracked paths the run is conservatively not cached. And a failed rename +over-collects its destination, which rule 7 permits. + +**Not an outcome.** Checking whether a path exists at archive time is a +filesystem query at decision time, not a syscall outcome, and it stays. It is +also what filters the over-collection above. diff --git a/crates/fspy/src/unix/syscall_handler/mod.rs b/crates/fspy/src/unix/syscall_handler/mod.rs index 4b6f7947e..722410779 100644 --- a/crates/fspy/src/unix/syscall_handler/mod.rs +++ b/crates/fspy/src/unix/syscall_handler/mod.rs @@ -57,14 +57,28 @@ impl SyscallHandler { } path = Cow::Owned(resolved_path); } - self.arena.add(PathAccess { - mode: match flags & libc::O_ACCMODE { - libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, - libc::O_WRONLY => AccessMode::WRITE, - _ => AccessMode::READ, - }, - path: path.as_os_str().into(), - }); + // This supervisor is notified before the syscall runs and responds with + // SECCOMP_USER_NOTIF_FLAG_CONTINUE, so it never learns the outcome. + // Only pre-call information is available here: intent plus the creation + // and truncation flags. Outcome-dependent flags (FAILED, DELETED, + // CREATED_DIR, RENAME_*) are deliberately not emitted, because guessing + // them would be worse than omitting them — a failed mkdir would claim a + // directory this run did not create. See DECISIONS.md D7. + let mut mode = match flags & libc::O_ACCMODE { + libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, + libc::O_WRONLY => AccessMode::WRITE, + _ => AccessMode::READ, + }; + if flags & libc::O_CREAT != 0 { + mode |= AccessMode::CREATE; + } + if flags & libc::O_TRUNC != 0 { + mode |= AccessMode::TRUNCATE; + } + if flags & libc::O_EXCL != 0 { + mode |= AccessMode::EXCLUSIVE; + } + self.arena.add(PathAccess { mode, path: path.as_os_str().into() }); Ok(()) } diff --git a/crates/fspy_preload_unix/src/client/mod.rs b/crates/fspy_preload_unix/src/client/mod.rs index 93d0d8153..d7511eeb2 100644 --- a/crates/fspy_preload_unix/src/client/mod.rs +++ b/crates/fspy_preload_unix/src/client/mod.rs @@ -158,22 +158,7 @@ pub unsafe fn handle_open(path: impl ToAbsolutePath, mode: impl ToAccessMode) { }); } -/// Report an access whose outcome is known, marking failures with -/// [`AccessMode::FAILED`]. -/// -/// Interceptions call this *after* forwarding to the real function. A failed -/// call still names the path, but it read nothing and wrote nothing, and -/// consumers must be able to tell the difference: a formatter probing for a -/// generated file that is not there yet is not depending on it. -pub unsafe fn handle_outcome( - path: impl ToAbsolutePath, - mode: fspy_shared::ipc::AccessMode, - succeeded: bool, -) { - let mode = if succeeded { mode } else { mode | fspy_shared::ipc::AccessMode::FAILED }; - // SAFETY: path contains valid pointers forwarded from the interposed function's caller - unsafe { handle_open(path, mode) }; -} + #[cfg(not(test))] #[ctor::ctor(unsafe)] diff --git a/crates/fspy_preload_unix/src/interceptions/access.rs b/crates/fspy_preload_unix/src/interceptions/access.rs index 8ba0ed146..0cae39220 100644 --- a/crates/fspy_preload_unix/src/interceptions/access.rs +++ b/crates/fspy_preload_unix/src/interceptions/access.rs @@ -1,21 +1,19 @@ -//! `access` and `faccessat` are pure existence and permission probes, so the -//! outcome is the whole signal. Reported after the call. - use fspy_shared::ipc::AccessMode; use libc::{c_char, c_int}; use crate::{ - client::{convert::PathAt, handle_outcome}, + client::{convert::PathAt, handle_open}, macros::intercept, }; intercept!(access(64): unsafe extern "C" fn(pathname: *const c_char, mode: c_int) -> c_int); unsafe extern "C" fn access(pathname: *const c_char, mode: c_int) -> c_int { - // SAFETY: calling the original libc access() with the same arguments forwarded from the interposed function - let result = unsafe { access::original()(pathname, mode) }; // SAFETY: pathname is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(pathname, AccessMode::READ, result == 0) }; - result + unsafe { + handle_open(pathname, AccessMode::READ); + } + // SAFETY: calling the original libc access() with the same arguments forwarded from the interposed function + unsafe { access::original()(pathname, mode) } } intercept!(faccessat(64): unsafe extern "C" fn(dirfd: c_int, pathname: *const c_char, mode: c_int, flags: c_int) -> c_int); @@ -25,9 +23,10 @@ unsafe extern "C" fn faccessat( mode: c_int, flags: c_int, ) -> c_int { - // SAFETY: calling the original libc faccessat() with the same arguments forwarded from the interposed function - let result = unsafe { faccessat::original()(dirfd, pathname, mode, flags) }; // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function - unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; - result + unsafe { + handle_open(PathAt(dirfd, pathname), AccessMode::READ); + } + // SAFETY: calling the original libc faccessat() with the same arguments forwarded from the interposed function + unsafe { faccessat::original()(dirfd, pathname, mode, flags) } } diff --git a/crates/fspy_preload_unix/src/interceptions/dirent.rs b/crates/fspy_preload_unix/src/interceptions/dirent.rs index 8fe45b120..d2b11d39a 100644 --- a/crates/fspy_preload_unix/src/interceptions/dirent.rs +++ b/crates/fspy_preload_unix/src/interceptions/dirent.rs @@ -2,7 +2,7 @@ use fspy_shared::ipc::AccessMode; use libc::{DIR, c_char, c_int, c_long, c_void}; use crate::{ - client::{convert::Fd, handle_outcome}, + client::{convert::Fd, handle_open}, macros::intercept, }; @@ -18,16 +18,15 @@ unsafe extern "C" fn scandir( select: *const c_void, compar: *const c_void, ) -> c_int { - // SAFETY: calling the original libc scandir() with the same arguments forwarded from the interposed function - let result = unsafe { scandir::original()(dirname, namelist, select, compar) }; // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(dirname, AccessMode::READ_DIR, result >= 0) }; - result + unsafe { handle_open(dirname, AccessMode::READ_DIR) } + // SAFETY: calling the original libc scandir() with the same arguments forwarded from the interposed function + unsafe { scandir::original()(dirname, namelist, select, compar) } } #[cfg(target_os = "macos")] mod macos_only { - use super::{AccessMode, Fd, c_char, c_int, c_void, handle_outcome, intercept}; + use super::{AccessMode, Fd, c_char, c_int, c_void, handle_open, intercept}; intercept!(scandir_b: unsafe extern "C" fn ( dirname: *const c_char, @@ -41,11 +40,10 @@ mod macos_only { select: *const c_void, compar: *const c_void, ) -> c_int { - // SAFETY: calling the original libc scandir_b() with the same arguments forwarded from the interposed function - let result = unsafe { scandir_b::original()(dirname, namelist, select, compar) }; // SAFETY: dirname is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(dirname, AccessMode::READ_DIR, result >= 0) }; - result + unsafe { handle_open(dirname, AccessMode::READ_DIR) }; + // SAFETY: calling the original libc scandir_b() with the same arguments forwarded from the interposed function + unsafe { scandir_b::original()(dirname, namelist, select, compar) } } intercept!(__getdirentries64: unsafe extern "C" fn(c_int, *mut u8, usize, *mut i64) -> isize); @@ -55,11 +53,10 @@ mod macos_only { buf_len: usize, basep: *mut i64, ) -> isize { - // SAFETY: calling the original libc __getdirentries64() with the same arguments forwarded from the interposed function - let result = unsafe { __getdirentries64::original()(fd, buf, buf_len, basep) }; // SAFETY: fd is a valid file descriptor provided by the caller of __getdirentries64 - unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, result >= 0) }; - result + unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; + // SAFETY: calling the original libc __getdirentries64() with the same arguments forwarded from the interposed function + unsafe { __getdirentries64::original()(fd, buf, buf_len, basep) } } } @@ -70,27 +67,24 @@ unsafe extern "C" fn getdirentries( nbytes: c_int, basep: *mut c_long, ) -> c_int { - // SAFETY: calling the original libc getdirentries() with the same arguments forwarded from the interposed function - let result = unsafe { getdirentries::original()(fd, buf, nbytes, basep) }; // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function - unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, result >= 0) }; - result + unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; + // SAFETY: calling the original libc getdirentries() with the same arguments forwarded from the interposed function + unsafe { getdirentries::original()(fd, buf, nbytes, basep) } } intercept!(fdopendir(64): unsafe extern "C" fn (fd: c_int) -> *mut DIR); unsafe extern "C" fn fdopendir(fd: c_int) -> *mut DIR { - // SAFETY: calling the original libc fdopendir() with the same arguments forwarded from the interposed function - let result = unsafe { fdopendir::original()(fd) }; // SAFETY: fd is a valid file descriptor provided by the caller of the interposed function - unsafe { handle_outcome(Fd(fd), AccessMode::READ_DIR, !result.is_null()) }; - result + unsafe { handle_open(Fd(fd), AccessMode::READ_DIR) }; + // SAFETY: calling the original libc fdopendir() with the same arguments forwarded from the interposed function + unsafe { fdopendir::original()(fd) } } intercept!(opendir(64): unsafe extern "C" fn (*const c_char) -> *mut DIR); unsafe extern "C" fn opendir(dir_name: *const c_char) -> *mut DIR { - // SAFETY: calling the original libc opendir() with the same arguments forwarded from the interposed function - let result = unsafe { opendir::original()(dir_name) }; // SAFETY: dir_name is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(dir_name, AccessMode::READ_DIR, !result.is_null()) }; - result + unsafe { handle_open(dir_name, AccessMode::READ_DIR) }; + // SAFETY: calling the original libc opendir() with the same arguments forwarded from the interposed function + unsafe { opendir::original()(dir_name) } } diff --git a/crates/fspy_preload_unix/src/interceptions/mutate.rs b/crates/fspy_preload_unix/src/interceptions/mutate.rs index eca40b718..c273b18c3 100644 --- a/crates/fspy_preload_unix/src/interceptions/mutate.rs +++ b/crates/fspy_preload_unix/src/interceptions/mutate.rs @@ -7,23 +7,30 @@ //! does. Directory renames are worse: a build that stages into `dist.tmp` and //! swaps it into place produces no write event for any of its real outputs. //! -//! All of these report *after* the real call, because only a successful call -//! changed anything. A failed `mkdir` is the common case, since most callers use -//! it as "ensure this exists" and expect `EEXIST`. +//! These report before the real call, like every other interception, and record +//! only what the arguments say. Nothing here depends on whether the call +//! succeeded: the Linux seccomp supervisor cannot observe outcomes at all, so an +//! outcome-dependent rule would hold on some backends and not others. +//! +//! `mkdir` is intentionally not intercepted. Its argument alone cannot say +//! whether this run created the directory, and callers overwhelmingly use it as +//! "ensure this exists" and ignore `EEXIST`, so recording it would claim every +//! pre-existing directory. use fspy_shared::ipc::AccessMode; use crate::{ - client::{convert::PathAt, handle_outcome}, + client::{convert::PathAt, handle_open}, libc::{c_char, c_int}, macros::intercept, }; -/// Whether a path is a directory, for tagging rename events. +/// Whether a path is a directory, for tagging rename and removal events. /// /// A consumer needs this to re-attribute writes recorded beneath a staging -/// directory to the published location. Called before the rename, while the -/// source still exists. +/// directory to the published location. Inspecting the source is not the same as +/// inspecting the rename's result: it describes an argument, and it happens +/// before the call while the source is still there. unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { let mut stat_buf: libc::stat = unsafe { core::mem::zeroed() }; // SAFETY: path is a valid C string pointer and stat_buf is a valid, owned stat struct @@ -31,28 +38,25 @@ unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { result == 0 && (stat_buf.st_mode & libc::S_IFMT) == libc::S_IFDIR } -/// Report both halves of a rename: the source is gone, the destination now -/// holds whatever the source held. +/// Report both halves of a rename: the source is being retired, the destination +/// is being replaced by whatever the source held. unsafe fn report_rename( old_dirfd: c_int, old_path: *const c_char, new_dirfd: c_int, new_path: *const c_char, - succeeded: bool, - was_directory: bool, + is_directory: bool, ) { - let dir_flag = if was_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + let dir_flag = if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; // SAFETY: old_path and new_path are valid C string pointers from the caller unsafe { - handle_outcome( + handle_open( PathAt(old_dirfd, old_path), AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag, - succeeded, ); - handle_outcome( + handle_open( PathAt(new_dirfd, new_path), AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, - succeeded, ); } } @@ -60,21 +64,13 @@ unsafe fn report_rename( intercept!(rename: unsafe extern "C" fn(*const c_char, *const c_char) -> c_int); unsafe extern "C" fn rename(old_path: *const c_char, new_path: *const c_char) -> c_int { // SAFETY: old_path is a valid C string pointer provided by the caller - let was_directory = unsafe { is_directory_at(libc::AT_FDCWD, old_path) }; - // SAFETY: forwarding the caller's arguments to the original libc rename() - let result = unsafe { rename::original()(old_path, new_path) }; - // SAFETY: both paths remain valid C string pointers after the call + let is_directory = unsafe { is_directory_at(libc::AT_FDCWD, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller unsafe { - report_rename( - libc::AT_FDCWD, - old_path, - libc::AT_FDCWD, - new_path, - result == 0, - was_directory, - ); + report_rename(libc::AT_FDCWD, old_path, libc::AT_FDCWD, new_path, is_directory); } - result + // SAFETY: forwarding the caller's arguments to the original libc rename() + unsafe { rename::original()(old_path, new_path) } } intercept!(renameat: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char) -> c_int); @@ -85,14 +81,11 @@ unsafe extern "C" fn renameat( new_path: *const c_char, ) -> c_int { // SAFETY: old_dirfd and old_path are valid arguments provided by the caller - let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; // SAFETY: forwarding the caller's arguments to the original libc renameat() - let result = unsafe { renameat::original()(old_dirfd, old_path, new_dirfd, new_path) }; - // SAFETY: both paths remain valid C string pointers after the call - unsafe { - report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); - } - result + unsafe { renameat::original()(old_dirfd, old_path, new_dirfd, new_path) } } // macOS publishes atomic swaps through renameatx_np with RENAME_SWAP, which @@ -108,15 +101,22 @@ unsafe extern "C" fn renameatx_np( flags: libc::c_uint, ) -> c_int { // SAFETY: old_dirfd and old_path are valid arguments provided by the caller - let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; - // SAFETY: forwarding the caller's arguments to the original renameatx_np() - let result = - unsafe { renameatx_np::original()(old_dirfd, old_path, new_dirfd, new_path, flags) }; - // SAFETY: both paths remain valid C string pointers after the call - unsafe { - report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; + // RENAME_SWAP mutates both sides, so the source is replaced too rather than + // simply retired. + if flags & u32::try_from(libc::RENAME_SWAP).unwrap_or(0) != 0 { + // SAFETY: old_path is a valid C string pointer provided by the caller + unsafe { + handle_open( + PathAt(old_dirfd, old_path), + AccessMode::RENAME_TO | AccessMode::WRITE, + ); + } } - result + // SAFETY: forwarding the caller's arguments to the original renameatx_np() + unsafe { renameatx_np::original()(old_dirfd, old_path, new_dirfd, new_path, flags) } } // Linux's renameat2 can also exchange two paths, in which case both sides are @@ -132,96 +132,54 @@ unsafe extern "C" fn renameat2( flags: libc::c_uint, ) -> c_int { // SAFETY: old_dirfd and old_path are valid arguments provided by the caller - let was_directory = unsafe { is_directory_at(old_dirfd, old_path) }; - // SAFETY: forwarding the caller's arguments to the original renameat2() - let result = unsafe { renameat2::original()(old_dirfd, old_path, new_dirfd, new_path, flags) }; - let exchanged = flags & libc::RENAME_EXCHANGE != 0; - // SAFETY: both paths remain valid C string pointers after the call - unsafe { - report_rename(old_dirfd, old_path, new_dirfd, new_path, result == 0, was_directory); - if exchanged { - // An exchange mutates the source too, so it is not simply gone. - handle_outcome( + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; + // RENAME_EXCHANGE mutates both sides, so the source is replaced too rather + // than simply retired. + if flags & libc::RENAME_EXCHANGE != 0 { + // SAFETY: old_path is a valid C string pointer provided by the caller + unsafe { + handle_open( PathAt(old_dirfd, old_path), AccessMode::RENAME_TO | AccessMode::WRITE, - result == 0, ); } } - result + // SAFETY: forwarding the caller's arguments to the original renameat2() + unsafe { renameat2::original()(old_dirfd, old_path, new_dirfd, new_path, flags) } } intercept!(unlink: unsafe extern "C" fn(*const c_char) -> c_int); unsafe extern "C" fn unlink(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED) }; // SAFETY: forwarding the caller's argument to the original libc unlink() - let result = unsafe { unlink::original()(path) }; - // SAFETY: path remains a valid C string pointer after the call - unsafe { handle_outcome(path, AccessMode::DELETED, result == 0) }; - result + unsafe { unlink::original()(path) } } intercept!(unlinkat: unsafe extern "C" fn(c_int, *const c_char, c_int) -> c_int); unsafe extern "C" fn unlinkat(dirfd: c_int, path: *const c_char, flags: c_int) -> c_int { + let dir_flag = + if flags & libc::AT_REMOVEDIR != 0 { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: dirfd and path are valid arguments provided by the caller + unsafe { handle_open(PathAt(dirfd, path), AccessMode::DELETED | dir_flag) }; // SAFETY: forwarding the caller's arguments to the original libc unlinkat() - let result = unsafe { unlinkat::original()(dirfd, path, flags) }; - let removed_directory = flags & libc::AT_REMOVEDIR != 0; - let dir_flag = if removed_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; - // SAFETY: path remains a valid C string pointer after the call - unsafe { - handle_outcome(PathAt(dirfd, path), AccessMode::DELETED | dir_flag, result == 0); - } - result + unsafe { unlinkat::original()(dirfd, path, flags) } } intercept!(remove: unsafe extern "C" fn(*const c_char) -> c_int); unsafe extern "C" fn remove(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED) }; // SAFETY: forwarding the caller's argument to the original libc remove() - let result = unsafe { remove::original()(path) }; - // SAFETY: path remains a valid C string pointer after the call - unsafe { handle_outcome(path, AccessMode::DELETED, result == 0) }; - result + unsafe { remove::original()(path) } } intercept!(rmdir: unsafe extern "C" fn(*const c_char) -> c_int); unsafe extern "C" fn rmdir(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED | AccessMode::IS_DIR) }; // SAFETY: forwarding the caller's argument to the original libc rmdir() - let result = unsafe { rmdir::original()(path) }; - // SAFETY: path remains a valid C string pointer after the call - unsafe { - handle_outcome(path, AccessMode::DELETED | AccessMode::IS_DIR, result == 0); - } - result -} - -intercept!(mkdir: unsafe extern "C" fn(*const c_char, libc::mode_t) -> c_int); -unsafe extern "C" fn mkdir(path: *const c_char, mode: libc::mode_t) -> c_int { - // SAFETY: forwarding the caller's arguments to the original libc mkdir() - let result = unsafe { mkdir::original()(path, mode) }; - // Only a successful mkdir means this run created the directory. Callers - // routinely ignore EEXIST, and treating that as creation would claim every - // pre-existing directory. - // SAFETY: path remains a valid C string pointer after the call - unsafe { - handle_outcome( - path, - AccessMode::CREATED_DIR | AccessMode::IS_DIR | AccessMode::WRITE, - result == 0, - ); - } - result -} - -intercept!(mkdirat: unsafe extern "C" fn(c_int, *const c_char, libc::mode_t) -> c_int); -unsafe extern "C" fn mkdirat(dirfd: c_int, path: *const c_char, mode: libc::mode_t) -> c_int { - // SAFETY: forwarding the caller's arguments to the original libc mkdirat() - let result = unsafe { mkdirat::original()(dirfd, path, mode) }; - // SAFETY: path remains a valid C string pointer after the call - unsafe { - handle_outcome( - PathAt(dirfd, path), - AccessMode::CREATED_DIR | AccessMode::IS_DIR | AccessMode::WRITE, - result == 0, - ); - } - result + unsafe { rmdir::original()(path) } } diff --git a/crates/fspy_preload_unix/src/interceptions/open.rs b/crates/fspy_preload_unix/src/interceptions/open.rs index 4d9be0fdb..641593a13 100644 --- a/crates/fspy_preload_unix/src/interceptions/open.rs +++ b/crates/fspy_preload_unix/src/interceptions/open.rs @@ -1,18 +1,9 @@ -//! Interceptions for the open family. -//! -//! These report *after* forwarding to the real function, so the recorded access -//! carries whether it succeeded. That distinction matters: a tool probing for a -//! generated file it is about to write is not depending on that file, and -//! counting the failed probe as a read makes a freshly generated output look -//! like an input it read before writing. - -use fspy_shared::ipc::AccessMode; use libc::FILE; use crate::{ client::{ - convert::{ModeStr, OpenFlags, PathAt, ToAccessMode as _}, - handle_outcome, + convert::{ModeStr, OpenFlags, PathAt}, + handle_open, }, libc::{c_char, c_int}, macros::intercept, @@ -34,28 +25,19 @@ type Mode = libc::mode_t; #[cfg(target_os = "macos")] // https://github.com/tailhook/openat/issues/21#issuecomment-535914957 type Mode = c_int; -/// The access mode implied by open flags, resolved before the call because the -/// flags are not affected by its outcome. -fn open_mode(flags: c_int) -> AccessMode { - // SAFETY: OpenFlags holds a plain integer, so no pointer is dereferenced - unsafe { OpenFlags(flags).to_access_mode() } -} - intercept!(open(64): unsafe extern "C" fn(*const c_char, c_int, args: ...) -> c_int); unsafe extern "C" fn open(path: *const c_char, flags: c_int, mut args: ...) -> c_int { - let mode = open_mode(flags); - let result = if has_mode_arg(flags) { + // SAFETY: path is a valid C string pointer provided by the caller of the interposed function + unsafe { handle_open(path, OpenFlags(flags)) }; + if has_mode_arg(flags) { // SAFETY: when O_CREAT or O_TMPFILE is set, a mode_t argument is required by the open() contract - let file_mode: Mode = unsafe { args.next_arg() }; + let mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc open() with the same arguments forwarded from the interposed function - unsafe { open::original()(path, flags, file_mode) } + unsafe { open::original()(path, flags, mode) } } else { // SAFETY: calling the original libc open() with the same arguments forwarded from the interposed function unsafe { open::original()(path, flags) } - }; - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(path, mode, result >= 0) }; - result + } } intercept!(openat(64): unsafe extern "C" fn(c_int, *const c_char, c_int, ...) -> c_int); @@ -65,39 +47,36 @@ unsafe extern "C" fn openat( flags: c_int, mut args: ... ) -> c_int { - let mode = open_mode(flags); - let result = if has_mode_arg(flags) { + // SAFETY: dirfd and path are valid arguments provided by the caller of the interposed function + unsafe { handle_open(PathAt(dirfd, path), OpenFlags(flags)) }; + + if has_mode_arg(flags) { // https://github.com/tailhook/openat/issues/21#issuecomment-535914957 // SAFETY: when O_CREAT or O_TMPFILE is set, a mode_t argument is required by the openat() contract - let file_mode: Mode = unsafe { args.next_arg() }; + let mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc openat() with the same arguments forwarded from the interposed function - unsafe { openat::original()(dirfd, path, flags, file_mode) } + unsafe { openat::original()(dirfd, path, flags, mode) } } else { // SAFETY: calling the original libc openat() with the same arguments forwarded from the interposed function unsafe { openat::original()(dirfd, path, flags) } - }; - // SAFETY: dirfd and path are valid arguments provided by the caller of the interposed function - unsafe { handle_outcome(PathAt(dirfd, path), mode, result >= 0) }; - result + } } #[cfg(target_os = "macos")] intercept!(open_nocancel: unsafe extern "C" fn(*const c_char, c_int, ...) -> c_int); #[cfg(target_os = "macos")] unsafe extern "C" fn open_nocancel(path: *const c_char, flags: c_int, mut args: ...) -> c_int { - let mode = open_mode(flags); - let result = if has_mode_arg(flags) { + // SAFETY: path is a valid C string pointer provided by the caller of open$NOCANCEL + unsafe { handle_open(path, OpenFlags(flags)) }; + if has_mode_arg(flags) { // SAFETY: O_CREAT requires a mode argument, matching the open$NOCANCEL contract - let file_mode: Mode = unsafe { args.next_arg() }; + let mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc open$NOCANCEL() with the same arguments forwarded from the interposed function - unsafe { open_nocancel::original()(path, flags, file_mode) } + unsafe { open_nocancel::original()(path, flags, mode) } } else { // SAFETY: calling the original libc open$NOCANCEL() with the same arguments forwarded from the interposed function unsafe { open_nocancel::original()(path, flags) } - }; - // SAFETY: path is a valid C string pointer provided by the caller of open$NOCANCEL - unsafe { handle_outcome(path, mode, result >= 0) }; - result + } } #[cfg(target_os = "macos")] @@ -109,30 +88,25 @@ unsafe extern "C" fn openat_nocancel( flags: c_int, mut args: ... ) -> c_int { - let mode = open_mode(flags); - let result = if has_mode_arg(flags) { + // SAFETY: dirfd and path are valid arguments provided by the caller of openat$NOCANCEL + unsafe { handle_open(PathAt(dirfd, path), OpenFlags(flags)) }; + if has_mode_arg(flags) { // SAFETY: O_CREAT requires a mode argument, matching the openat$NOCANCEL contract - let file_mode: Mode = unsafe { args.next_arg() }; + let mode: Mode = unsafe { args.next_arg() }; // SAFETY: calling the original libc openat$NOCANCEL() with the same arguments forwarded from the interposed function - unsafe { openat_nocancel::original()(dirfd, path, flags, file_mode) } + unsafe { openat_nocancel::original()(dirfd, path, flags, mode) } } else { // SAFETY: calling the original libc openat$NOCANCEL() with the same arguments forwarded from the interposed function unsafe { openat_nocancel::original()(dirfd, path, flags) } - }; - // SAFETY: dirfd and path are valid arguments provided by the caller of openat$NOCANCEL - unsafe { handle_outcome(PathAt(dirfd, path), mode, result >= 0) }; - result + } } intercept!(fopen(64): unsafe extern "C" fn(path: *const c_char, mode: *const c_char) -> *mut FILE); unsafe extern "C" fn fopen(path: *const c_char, mode: *const c_char) -> *mut libc::FILE { - // SAFETY: mode is a valid C string pointer provided by the caller of the interposed function - let access_mode = unsafe { ModeStr(mode).to_access_mode() }; + // SAFETY: path and mode are valid C string pointers provided by the caller of the interposed function + unsafe { handle_open(path, ModeStr(mode)) }; // SAFETY: calling the original libc fopen() with the same arguments forwarded from the interposed function - let result = unsafe { fopen::original()(path, mode) }; - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(path, access_mode, !result.is_null()) }; - result + unsafe { fopen::original()(path, mode) } } intercept!(freopen(64): unsafe extern "C" fn(path: *const c_char, mode: *const c_char, stream: *mut FILE) -> *mut FILE); @@ -141,11 +115,8 @@ unsafe extern "C" fn freopen( mode: *const c_char, stream: *mut FILE, ) -> *mut FILE { - // SAFETY: mode is a valid C string pointer provided by the caller of the interposed function - let access_mode = unsafe { ModeStr(mode).to_access_mode() }; + // SAFETY: path and mode are valid C string pointers provided by the caller of the interposed function + unsafe { handle_open(path, ModeStr(mode)) }; // SAFETY: calling the original libc freopen() with the same arguments forwarded from the interposed function - let result = unsafe { freopen::original()(path, mode, stream) }; - // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(path, access_mode, !result.is_null()) }; - result + unsafe { freopen::original()(path, mode, stream) } } diff --git a/crates/fspy_preload_unix/src/interceptions/stat.rs b/crates/fspy_preload_unix/src/interceptions/stat.rs index e31bdd278..ac4af7651 100644 --- a/crates/fspy_preload_unix/src/interceptions/stat.rs +++ b/crates/fspy_preload_unix/src/interceptions/stat.rs @@ -4,27 +4,29 @@ use libc::{c_char, c_int, stat as stat_struct}; #[cfg(target_os = "linux")] use crate::client::convert::Fd; use crate::{ - client::{convert::PathAt, handle_outcome}, + client::{convert::PathAt, handle_open}, macros::intercept, }; intercept!(stat(64): unsafe extern "C" fn(path: *const c_char, buf: *mut stat_struct) -> c_int); unsafe extern "C" fn stat(path: *const c_char, buf: *mut stat_struct) -> c_int { - // SAFETY: calling the original libc stat() with the same arguments forwarded from the interposed function - let result = unsafe { stat::original()(path, buf) }; // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(path, AccessMode::READ, result == 0) }; - result + unsafe { + handle_open(path, AccessMode::READ); + } + // SAFETY: calling the original libc stat() with the same arguments forwarded from the interposed function + unsafe { stat::original()(path, buf) } } intercept!(lstat(64): unsafe extern "C" fn(path: *const c_char, buf: *mut stat_struct) -> c_int); unsafe extern "C" fn lstat(path: *const c_char, buf: *mut stat_struct) -> c_int { // TODO: add accessmode ReadNoFollow - // SAFETY: calling the original libc lstat() with the same arguments forwarded from the interposed function - let result = unsafe { lstat::original()(path, buf) }; // SAFETY: path is a valid C string pointer provided by the caller of the interposed function - unsafe { handle_outcome(path, AccessMode::READ, result == 0) }; - result + unsafe { + handle_open(path, AccessMode::READ); + } + // SAFETY: calling the original libc lstat() with the same arguments forwarded from the interposed function + unsafe { lstat::original()(path, buf) } } intercept!(fstatat(64): unsafe extern "C" fn(dirfd: c_int, pathname: *const c_char, buf: *mut stat_struct, flags: c_int) -> c_int); @@ -34,11 +36,12 @@ unsafe extern "C" fn fstatat( buf: *mut stat_struct, flags: c_int, ) -> c_int { - // SAFETY: calling the original libc fstatat() with the same arguments forwarded from the interposed function - let result = unsafe { fstatat::original()(dirfd, pathname, buf, flags) }; // SAFETY: dirfd and pathname are valid arguments provided by the caller of the interposed function - unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; - result + unsafe { + handle_open(PathAt(dirfd, pathname), AccessMode::READ); + } + // SAFETY: calling the original libc fstatat() with the same arguments forwarded from the interposed function + unsafe { fstatat::original()(dirfd, pathname, buf, flags) } } #[cfg(target_os = "linux")] @@ -65,16 +68,15 @@ unsafe extern "C" fn statx( return -1; }; - // SAFETY: calling the original libc statx() with the same arguments forwarded from the interposed function - let result = unsafe { original(dirfd, pathname, flags, mask, statxbuf) }; if pathname.is_null() { if flags & libc::AT_EMPTY_PATH != 0 { // SAFETY: dirfd is provided by the statx caller. - unsafe { handle_outcome(Fd(dirfd), AccessMode::READ, result == 0) }; + unsafe { handle_open(Fd(dirfd), AccessMode::READ) }; } } else { // SAFETY: pathname is a non-null C string pointer provided by the statx caller. - unsafe { handle_outcome(PathAt(dirfd, pathname), AccessMode::READ, result == 0) }; + unsafe { handle_open(PathAt(dirfd, pathname), AccessMode::READ) }; } - result + // SAFETY: calling the original libc statx() with the same arguments forwarded from the interposed function + unsafe { original(dirfd, pathname, flags, mask, statxbuf) } } diff --git a/crates/fspy_shared/src/ipc/mod.rs b/crates/fspy_shared/src/ipc/mod.rs index a21fb34ae..92ecb4420 100644 --- a/crates/fspy_shared/src/ipc/mod.rs +++ b/crates/fspy_shared/src/ipc/mod.rs @@ -12,16 +12,20 @@ use wincode::{SchemaRead, SchemaWrite}; pub struct AccessMode(u16); bitflags! { - /// What a process did, or tried to do, to one path. + /// What a process asked the kernel to do with one path. /// - /// `READ`, `WRITE` and `READ_DIR` describe the *intent* of the call. The - /// remaining flags qualify it, and consumers need them to tell a real - /// dependency from a probe and a real mutation from mere write capability: + /// Every flag here is derived from the call's *arguments*, never from its + /// result. A tracer that reported outcomes could not behave the same on all + /// backends: the Linux seccomp supervisor is notified before the syscall + /// runs and never learns whether it worked. Restricting the vocabulary to + /// pre-call information keeps every backend equivalent, and makes a trace a + /// function of the arguments alone rather than of filesystem state at trace + /// time. /// - /// - a `READ` that also has `FAILED` named a path that was not there, so it - /// is an absence check rather than a dependency; - /// - a `WRITE` on its own is only capability. Truncating, exclusively - /// creating, or being a rename destination is what makes it a mutation. + /// `READ`, `WRITE` and `READ_DIR` are intent. The rest qualify it, and + /// consumers need them because `WRITE` on its own is only capability: + /// truncating, exclusively creating, or being a rename destination is what + /// makes an access a mutation. impl AccessMode: u16 { /// Opened for reading, or its metadata was inspected. const READ = 1; @@ -29,59 +33,52 @@ bitflags! { const WRITE = 1 << 1; /// Directory entries were listed. const READ_DIR = 1 << 2; - /// The call failed. The path was still named, but nothing was read, - /// written or listed. - const FAILED = 1 << 3; /// The open carried `O_CREAT`. - const CREATE = 1 << 4; - /// The open carried `O_TRUNC`, so any previous contents are gone. - const TRUNCATE = 1 << 5; - /// The open carried `O_EXCL`, so the path is newly created and had no - /// previous contents to read. - const EXCLUSIVE = 1 << 6; - /// The path was the source of a successful rename and no longer exists - /// under this name. - const RENAME_FROM = 1 << 7; - /// The path was the destination of a successful rename, which replaced - /// whatever was there. - const RENAME_TO = 1 << 8; - /// The path was successfully removed by `unlink`, `remove` or `rmdir`. - const DELETED = 1 << 9; - /// A directory was successfully created at this path. - const CREATED_DIR = 1 << 10; - /// The path is a directory. Set on rename events so a consumer can - /// re-attribute writes recorded beneath a renamed directory. - const IS_DIR = 1 << 11; + const CREATE = 1 << 3; + /// The open carried `O_TRUNC`, so any previous contents are discarded. + const TRUNCATE = 1 << 4; + /// The open carried `O_EXCL`, so the path is new by construction and had + /// no previous contents to read. + const EXCLUSIVE = 1 << 5; + /// The path was named as the source of a rename. + const RENAME_FROM = 1 << 6; + /// The path was named as the destination of a rename, which replaces + /// whatever is there. + const RENAME_TO = 1 << 7; + /// Removal was requested through `unlink`, `remove` or `rmdir`. + const DELETED = 1 << 8; + /// The path is a directory. Set on rename and removal events so a + /// consumer can re-attribute writes recorded beneath a renamed + /// directory. + const IS_DIR = 1 << 9; } } impl AccessMode { - /// Whether this access changed the path's contents. + /// Whether this access changes the path's contents. /// - /// Write capability alone does not qualify: a descriptor opened `O_RDWR` and + /// Write capability alone does not qualify. A descriptor opened `O_RDWR` and /// never written leaves the file untouched, which is what Biome does to a - /// clean source file on a warm run. + /// clean source file on a warm run, and what every `flock`-only lock file + /// looks like. #[must_use] pub const fn is_mutation(self) -> bool { - if self.contains(Self::FAILED) { - return false; - } - if self.intersects(Self::RENAME_TO.union(Self::DELETED).union(Self::CREATED_DIR)) { + if self.contains(Self::RENAME_TO) { return true; } self.contains(Self::WRITE) && self.intersects(Self::TRUNCATE.union(Self::CREATE.union(Self::EXCLUSIVE))) } - /// Whether this access observed existing content, so the path is a genuine - /// dependency. + /// Whether this access can observe existing content, making the path a + /// genuine dependency. /// - /// A failed call observed nothing. An exclusive create observed nothing - /// either, because the path is new by definition, which is how Go's - /// `os.CreateTemp` opens atomic-write temporaries. + /// An exclusive create observes nothing, because the path is new by + /// definition. That is how Go's `os.CreateTemp` opens atomic-write + /// temporaries, which would otherwise look read-first. #[must_use] pub const fn is_content_read(self) -> bool { - if self.contains(Self::FAILED) || self.contains(Self::EXCLUSIVE) { + if self.contains(Self::EXCLUSIVE) { return false; } self.intersects(Self::READ.union(Self::READ_DIR)) diff --git a/crates/vite_task/Cargo.toml b/crates/vite_task/Cargo.toml index 77a9771ef..70756f3f4 100644 --- a/crates/vite_task/Cargo.toml +++ b/crates/vite_task/Cargo.toml @@ -20,6 +20,7 @@ clap = { workspace = true, features = ["derive"] } ctrlc = { workspace = true } derive_more = { workspace = true, features = ["debug", "from"] } futures-util = { workspace = true } +ignore = { workspace = true } once_cell = { workspace = true } owo-colors = { workspace = true } petgraph = { workspace = true } diff --git a/crates/vite_task/src/session/cache/mod.rs b/crates/vite_task/src/session/cache/mod.rs index b18bccaca..0b0b793fa 100644 --- a/crates/vite_task/src/session/cache/mod.rs +++ b/crates/vite_task/src/session/cache/mod.rs @@ -274,7 +274,7 @@ pub fn split_path(path: &str) -> (Option<&str>, &str) { /// its own cache warm across branch switches, and a cache from a different /// version is simply ignored (it lives in a directory this build never looks /// at) rather than aborting the run. Bumping the version starts a fresh cache. -const CACHE_SCHEMA_VERSION: u32 = 18; +const CACHE_SCHEMA_VERSION: u32 = 19; /// Name of the per-version subdirectory (e.g. `v14`) under the task-cache /// directory that holds the database and output archives for the current diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index 0cf4df1c5..6b36dfea0 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -11,12 +11,11 @@ use vite_task_server::Reports; use super::{ CacheState, - fingerprint::{PathRead, PostRunFingerprint, TrackedEnvQuery}, + fingerprint::{PostRunFingerprint, TrackedEnvQuery}, glob, spawn::ChildOutcome, }; use crate::{ - collections::HashMap, session::{ cache::{CacheEntryValue, ExecutionCache, archive}, event::{CacheErrorKind, CacheNotUpdatedReason, CacheUpdateStatus, ExecutionError}, @@ -27,12 +26,13 @@ use crate::{ /// cfg-agnostic so the decision logic below doesn't need `cfg(fspy)` — the /// value is only ever `Some` when tracking happened (see [`observe_fspy`]). struct TrackingOutcome { - path_reads: HashMap, + /// Paths to fingerprint after the run. + path_reads: FxHashSet, /// Auto-output writes after output exclusions are applied. Empty when /// `output_config.includes_auto` is false. path_writes: FxHashSet, - /// First path that was both read and written during execution, if any. - /// A non-empty value means caching this task is unsound. + /// A tracked path this run read and then rewrote. The prerun fingerprint no + /// longer describes it, so caching this task is unsound. read_write_overlap: Option, } @@ -135,7 +135,7 @@ pub(super) async fn update_cache( // Paths already in globbed_inputs are skipped: the overlap check above // guarantees no input modification, so the prerun hash is the correct // post-exec hash. - let empty_path_reads = HashMap::default(); + let empty_path_reads = FxHashSet::default(); let path_reads = fspy_outcome.as_ref().map_or(&empty_path_reads, |o| &o.path_reads); let post_run_fingerprint = match PostRunFingerprint::create( path_reads, @@ -203,53 +203,70 @@ fn observe_fspy( ) -> Option { #[cfg(fspy)] { - use super::tracked_accesses::TrackedPathAccesses; + use super::{ + classify::{ClassifyContext, classify}, + tracked_accesses::TrackedPathAccesses, + }; outcome.path_accesses.as_ref().map(|raw| { let tracked = TrackedPathAccesses::from_raw(raw, workspace_root); - let filtered_path_reads: HashMap = - // fspy can be attached for auto-output-only tasks. In that - // mode reads must not become inferred inputs. - if metadata.input_config.includes_auto - && let Some(fspy) = fspy - { - tracked - .path_reads - .iter() - .filter(|(path, _)| { - !fspy.input_negative_globs.is_match(path.as_str()) - && !is_ignored(path, ignored_input_rels) - }) - .map(|(path, read)| (path.clone(), *read)) - .collect() - } else { - HashMap::default() - }; - let filtered_path_writes: FxHashSet = - // fspy can also be attached for auto-input-only tasks. In that - // mode writes must not become auto outputs or overlap candidates. - if metadata.output_config.includes_auto - && let Some(fspy) = fspy - { - tracked - .path_writes - .iter() - .filter(|path| { - !fspy.output_negative_globs.is_match(path.as_str()) - && !is_ignored(path, ignored_output_rels) - }) - .cloned() - .collect() - } else { - FxHashSet::default() - }; - let read_write_overlap = - filtered_path_reads.keys().find(|p| filtered_path_writes.contains(*p)).cloned(); - TrackingOutcome { - path_reads: filtered_path_reads, - path_writes: filtered_path_writes, - read_write_overlap, - } + + // fspy can be attached for auto-output-only or auto-input-only + // tasks. In those modes the other side must stay empty. + let infer_inputs = metadata.input_config.includes_auto && fspy.is_some(); + let infer_outputs = metadata.output_config.includes_auto && fspy.is_some(); + + let gitignore = super::gitignore::WorkspaceGitignore::open(workspace_root); + let is_gitignored = |path: &RelativePathBuf| gitignore.is_ignored(path); + + let context = ClassifyContext { + workspace_root, + infer_inputs, + infer_outputs, + is_gitignored: &is_gitignored, + }; + + // Negative globs and tool-reported ignores are applied to the + // classified result rather than to raw events, because a path + // excluded from inputs may still be a legitimate output and vice + // versa. + let classification = classify(&tracked.events, &context, &|_| false); + + let excluded_from_inputs = |path: &RelativePathBuf| { + fspy.is_some_and(|fspy| fspy.input_negative_globs.is_match(path.as_str())) + || is_ignored(path, ignored_input_rels) + }; + let excluded_from_outputs = |path: &RelativePathBuf| { + fspy.is_some_and(|fspy| fspy.output_negative_globs.is_match(path.as_str())) + || is_ignored(path, ignored_output_rels) + }; + + let path_reads: FxHashSet = classification + .inputs + .iter() + .filter(|path| !excluded_from_inputs(path)) + .cloned() + .collect(); + + let path_writes: FxHashSet = classification + .outputs + .iter() + .filter(|path| !excluded_from_outputs(path)) + .cloned() + .collect(); + + // A modified input only blocks caching if it is still tracked as an + // input after exclusions; excluding it is the user saying they know. + let read_write_overlap = classification + .modified_input + .filter(|path| !excluded_from_inputs(path)) + .or_else(|| { + classification + .unexplained_mutation + .filter(|path| !excluded_from_outputs(path)) + }); + + TrackingOutcome { path_reads, path_writes, read_write_overlap } }) } #[cfg(not(fspy))] diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs new file mode 100644 index 000000000..df2ff8a26 --- /dev/null +++ b/crates/vite_task/src/session/execute/classify.rs @@ -0,0 +1,287 @@ +//! Decide which observed paths a task consumed and which it produced. +//! +//! Two problems are solved here. +//! +//! **Read-write overlaps.** A path a task both read and wrote is either a source +//! it rewrote or state it manages, and the two need opposite treatment. Access +//! mechanics cannot tell them apart: a formatter rewriting a source and a linter +//! rewriting its own cache both read, truncate and write. So order decides +//! first — a task that wrote before reading is observing its own output — and +//! where order does not settle it, version control does. A tracked file the task +//! rewrote is a modified input; an ignored one is derived state. +//! +//! **Output directory cleanup.** Build tools stat and list their output +//! directory before writing to it, which would make the directory an input whose +//! content is the task's own product. Such a fingerprint can never match on a +//! clean checkout, because the directory is not there. Two rules drop those +//! reads: a bare directory stat carries only existence and is ignored, and a +//! directory whose subtree this task wrote into is the task's own product rather +//! than something it consumed. +//! +//! Nothing here consults whether a call succeeded. Only the call's arguments are +//! available, because the Linux seccomp supervisor is notified before the +//! syscall runs and can never report an outcome; a rule built on outcomes would +//! hold on some backends and silently not on others. +//! +//! Explicit input and output globs are applied by the caller and always win; +//! this module only classifies what the user did not declare. + +#![cfg(fspy)] + +use fspy::AccessMode; +use rustc_hash::{FxHashMap, FxHashSet}; +use vite_path::{AbsolutePath, RelativePathBuf}; + +/// One normalized, workspace-relative access, in the order it was observed. +#[derive(Debug)] +pub struct TrackedEvent { + pub path: RelativePathBuf, + pub mode: AccessMode, +} + +/// What the caller must supply for the parts of the decision that do not come +/// from access data. +pub struct ClassifyContext<'a> { + pub workspace_root: &'a AbsolutePath, + /// Whether reads may become inferred inputs. + pub infer_inputs: bool, + /// Whether writes may become inferred outputs. + pub infer_outputs: bool, + /// Version control's answer for a path, used only to break a read-first + /// overlap. `None` means not ignored, or no repository, which resolves to + /// input. + pub is_gitignored: &'a dyn Fn(&RelativePathBuf) -> bool, +} + +/// The classification result. +#[derive(Default)] +pub struct Classification { + /// Paths to fingerprint. A path present here is a dependency of the task. + pub inputs: FxHashSet, + /// Of those inputs, the ones whose directory entries were listed and so + /// must be fingerprinted as a listing rather than as content. + pub listed_directories: FxHashSet, + /// Paths to archive as this task's outputs. + pub outputs: FxHashSet, + /// A tracked path the task rewrote after reading it, so the prerun + /// fingerprint is stale and this run must not be cached. + pub modified_input: Option, + /// A mutation whose target is gone at archive time with no rename to explain + /// it. The output set is incomplete, so this run must not be cached. + pub unexplained_mutation: Option, +} + +/// Everything observed about one path, folded in observation order. +#[derive(Default)] +struct PathHistory { + /// Index of the first access that observed existing content. + first_content_read: Option, + /// Index of the first access that changed the path. + first_mutation: Option, + /// Index of the first successful directory listing. + first_listing: Option, + /// Index of the first successful removal. + first_deletion: Option, + /// The path was named as a rename source, so its absence is explained. + renamed_away: bool, + /// The path was replaced by a rename. + rename_destination: bool, + /// Any access described the path as a directory. + known_directory: bool, +} + +impl PathHistory { + fn observe(&mut self, index: usize, mode: AccessMode) { + if mode.contains(AccessMode::IS_DIR) { + self.known_directory = true; + } + if mode.contains(AccessMode::RENAME_FROM) { + self.renamed_away = true; + } + if mode.contains(AccessMode::RENAME_TO) { + self.rename_destination = true; + } + if mode.contains(AccessMode::DELETED) && self.first_deletion.is_none() { + self.first_deletion = Some(index); + } + if mode.contains(AccessMode::READ_DIR) && self.first_listing.is_none() { + self.first_listing = Some(index); + } + if mode.is_content_read() && self.first_content_read.is_none() { + self.first_content_read = Some(index); + } + if mode.is_mutation() && self.first_mutation.is_none() { + self.first_mutation = Some(index); + } + } +} + +/// Apply the rules to an ordered event stream. +/// +/// `already_fingerprinted` reports paths the caller hashed before the run, which +/// do not need fingerprinting again. +pub fn classify( + events: &[TrackedEvent], + context: &ClassifyContext<'_>, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) -> Classification { + let mut histories: FxHashMap<&RelativePathBuf, PathHistory> = FxHashMap::default(); + // Directory renames, newest last. A build that stages into `dist.tmp` and + // swaps it into place records every write against the staging path, so those + // writes have to be re-attributed or the real outputs are lost entirely. + let mut directory_renames: Vec<(&RelativePathBuf, &RelativePathBuf)> = Vec::new(); + let mut last_rename_source: Option<&RelativePathBuf> = None; + + for (index, event) in events.iter().enumerate() { + histories.entry(&event.path).or_default().observe(index, event.mode); + + // The preload reports a rename as source then destination, adjacent. + if event.mode.contains(AccessMode::RENAME_FROM) { + last_rename_source = Some(&event.path); + } else if event.mode.contains(AccessMode::RENAME_TO) { + if event.mode.contains(AccessMode::IS_DIR) + && let Some(source) = last_rename_source + { + directory_renames.push((source, &event.path)); + } + last_rename_source = None; + } + } + + let mut classification = Classification::default(); + + // Re-attribute mutations recorded beneath a renamed directory. + let mut relocated: FxHashMap = FxHashMap::default(); + for (source, destination) in &directory_renames { + let source_prefix = source.as_str(); + for (path, history) in &histories { + let Some(tail) = path.as_str().strip_prefix(source_prefix) else { + continue; + }; + let Some(tail) = tail.strip_prefix('/') else { + continue; + }; + if history.first_mutation.is_none() { + continue; + } + let Ok(moved) = RelativePathBuf::new(vite_str::format!("{destination}/{tail}").as_str()) + else { + continue; + }; + relocated.entry(moved).or_default().first_mutation = history.first_mutation; + } + } + + let mut candidates: Vec<(&RelativePathBuf, &PathHistory)> = + histories.iter().map(|(path, history)| (*path, history)).collect(); + let relocated_entries: Vec<(&RelativePathBuf, &PathHistory)> = + relocated.iter().map(|(path, history)| (path, history)).collect(); + candidates.extend(relocated_entries); + + // Every directory this task wrote into, including ancestors. A listing of + // one of these enumerated the task's own product, so treating it as a + // dependency would make the cache key depend on the output and could never + // match on a clean checkout, where the directory does not exist yet. + let mut mutated_subtrees: FxHashSet<&str> = FxHashSet::default(); + for (path, history) in &candidates { + if history.first_mutation.is_none() { + continue; + } + let mut remaining = path.as_str(); + while let Some(separator) = remaining.rfind('/') { + remaining = &remaining[..separator]; + if !mutated_subtrees.insert(remaining) { + // An ancestor already recorded, so the rest are too. + break; + } + } + } + + for (path, history) in candidates { + let absolute = context.workspace_root.join(path); + let metadata = std::fs::metadata(absolute.as_path()).ok(); + let exists = metadata.is_some(); + let is_directory = metadata.as_ref().is_some_and(std::fs::Metadata::is_dir) + || (history.known_directory && !exists); + + if history.first_mutation.is_some() { + let cleaned_up = history.first_deletion.is_some() && !exists; + if !exists && !history.renamed_away && !cleaned_up { + // Something changed and then vanished with nothing to explain + // where it went. Whatever produced it is not in the archive, so + // caching this run could restore an incomplete tree. + classification.unexplained_mutation.get_or_insert_with(|| path.clone()); + } + } + + let wrote_into_subtree = is_directory && mutated_subtrees.contains(path.as_str()); + let mutated = history.first_mutation.is_some() && context.infer_outputs; + let consumed = + is_input_candidate(history, is_directory, wrote_into_subtree) && context.infer_inputs; + + match (mutated, consumed) { + (true, false) => { + if exists { + classification.outputs.insert(path.clone()); + } + } + (true, true) => { + let write_first = history.first_mutation < history.first_content_read; + if write_first || (context.is_gitignored)(path) { + if exists { + classification.outputs.insert(path.clone()); + } + } else { + // A tracked file this run read and then rewrote. The prerun + // fingerprint no longer describes it. + classification.modified_input.get_or_insert_with(|| path.clone()); + record_input(&mut classification, path, history, already_fingerprinted); + } + } + (false, true) => record_input(&mut classification, path, history, already_fingerprinted), + (false, false) => {} + } + } + + classification +} + +/// Whether a path's reads make it a dependency. +/// +/// Directories are the interesting case. A bare stat of one carries only +/// existence and type, which cannot detect a content change and is routinely a +/// probe of the task's own output directory. A listing is a real dependency, +/// unless the task wrote into that directory, in which case the entries it saw +/// are its own product. +fn is_input_candidate( + history: &PathHistory, + is_directory: bool, + wrote_into_subtree: bool, +) -> bool { + if history.first_content_read.is_none() { + return false; + } + if !is_directory { + return true; + } + if history.first_listing.is_none() { + // Stat only. Existence alone is not worth a fingerprint, and a stored + // one can never match on a clean checkout. + return false; + } + !wrote_into_subtree +} + +fn record_input( + classification: &mut Classification, + path: &RelativePathBuf, + history: &PathHistory, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) { + if history.first_listing.is_some() { + classification.listed_directories.insert(path.clone()); + } + if !already_fingerprinted(path) { + classification.inputs.insert(path.clone()); + } +} diff --git a/crates/vite_task/src/session/execute/fingerprint.rs b/crates/vite_task/src/session/execute/fingerprint.rs index 11173d40e..2129e1bde 100644 --- a/crates/vite_task/src/session/execute/fingerprint.rs +++ b/crates/vite_task/src/session/execute/fingerprint.rs @@ -32,12 +32,6 @@ pub enum TrackedEnvQuery { Prefix(Str), } -/// Path read access info -#[derive(Debug, Clone, Copy)] -pub struct PathRead { - pub read_dir_entries: bool, -} - /// Post-run fingerprint capturing file state after execution. /// Used to validate whether cached outputs are still valid. #[derive(SchemaWrite, SchemaRead, Debug, Default, Serialize)] @@ -85,11 +79,13 @@ pub enum PathFingerprint { NotFound, /// File content hash using `xxHash3_64` FileContentHash(u64), - /// Directory with optional entry listing. - /// `Folder(None)` means the directory was opened but entries were not read - /// (e.g., for `openat` calls). - /// `Folder(Some(_))` contains the directory entries sorted by name. - Folder(Option>), + /// Directory entries, sorted by name. + /// + /// A directory only reaches here if the task listed it. A bare stat carries + /// nothing but existence, cannot detect a content change, and is routinely a + /// probe of the task's own output directory, so classification drops those + /// before fingerprinting. + Folder(BTreeMap), } /// Kind of directory entry @@ -118,7 +114,7 @@ impl PostRunFingerprint { /// * `tracked_env_queries` - Tool-requested bulk env queries (query -> match-set hashes) #[tracing::instrument(level = "debug", skip_all, name = "create_post_run_fingerprint")] pub fn create( - inferred_path_reads: &HashMap, + inferred_path_reads: &rustc_hash::FxHashSet, base_dir: &AbsolutePath, globbed_inputs: &BTreeMap, tracked_envs: BTreeMap>, @@ -126,10 +122,10 @@ impl PostRunFingerprint { ) -> anyhow::Result { let inferred_inputs = inferred_path_reads .par_iter() - .filter(|(path, _)| !globbed_inputs.contains_key(*path)) - .map(|(relative_path, path_read)| { + .filter(|path| !globbed_inputs.contains_key(*path)) + .map(|relative_path| { let full_path = Arc::::from(base_dir.join(relative_path)); - let fingerprint = fingerprint_path(&full_path, *path_read)?; + let fingerprint = fingerprint_path(&full_path)?; Ok((relative_path.clone(), fingerprint)) }) .collect::>>()?; @@ -155,10 +151,7 @@ impl PostRunFingerprint { let input_mismatch = self.inferred_inputs.par_iter().find_map_any( |(input_relative_path, path_fingerprint)| { let input_full_path = Arc::::from(base_dir.join(input_relative_path)); - let path_read = PathRead { - read_dir_entries: matches!(path_fingerprint, PathFingerprint::Folder(Some(_))), - }; - let current_path_fingerprint = match fingerprint_path(&input_full_path, path_read) { + let current_path_fingerprint = match fingerprint_path(&input_full_path) { Ok(ok) => ok, Err(err) => return Some(Err(err)), }; @@ -337,7 +330,7 @@ fn determine_change_kind<'a>( (InputChangeKind::ContentModified, None) } (PathFingerprint::Folder(old), PathFingerprint::Folder(new)) => { - determine_folder_change_kind(old.as_ref(), new.as_ref()) + determine_folder_change_kind(old, new) } // Type changed (file ↔ folder) _ => (InputChangeKind::Added, None), @@ -348,13 +341,9 @@ fn determine_change_kind<'a>( /// Both maps are `BTreeMap` so we iterate them in sorted lockstep. /// Returns the specific entry name that was added or removed, if identifiable. fn determine_folder_change_kind<'a>( - old: Option<&'a BTreeMap>, - new: Option<&'a BTreeMap>, + old_entries: &'a BTreeMap, + new_entries: &'a BTreeMap, ) -> (InputChangeKind, Option<&'a Str>) { - let (Some(old_entries), Some(new_entries)) = (old, new) else { - return (InputChangeKind::Added, None); - }; - let mut old_iter = old_entries.iter(); let mut new_iter = new_entries.iter(); let mut o = old_iter.next(); @@ -386,10 +375,7 @@ fn should_ignore_entry(name: &[u8]) -> bool { } /// Fingerprint a single path -pub fn fingerprint_path( - path: &Arc, - path_read: PathRead, -) -> anyhow::Result { +pub fn fingerprint_path(path: &Arc) -> anyhow::Result { let std_path = path.as_path(); let file = match File::open(std_path) { @@ -400,12 +386,12 @@ pub fn fingerprint_path( { if err.kind() == io::ErrorKind::PermissionDenied { // This might be a directory - try reading it as such - return process_directory(std_path, path_read); + return process_directory(std_path); } // On Windows, paths with trailing backslash (from joining empty path) // fail with NotFound (error code 3). Try as directory in this case. if err.raw_os_error() == Some(3) && std_path.to_string_lossy().ends_with('\\') { - return process_directory(std_path, path_read); + return process_directory(std_path); } } if err.kind() != io::ErrorKind::NotFound { @@ -428,11 +414,11 @@ pub fn fingerprint_path( // Is a directory on Unix - use the optimized nix implementation #[cfg(unix)] { - return process_directory_unix(reader.get_ref(), path_read); + return process_directory_unix(reader.get_ref()); } #[cfg(windows)] { - return process_directory(std_path, path_read); + return process_directory(std_path); } } Ok(PathFingerprint::FileContentHash(super::hash::hash_content(reader)?)) @@ -441,14 +427,7 @@ pub fn fingerprint_path( /// Process a directory on Windows using `std::fs::read_dir` #[cfg(windows)] #[expect(clippy::disallowed_types, reason = "Windows fallback uses std::path::Path directly")] -fn process_directory( - path: &std::path::Path, - path_read: PathRead, -) -> anyhow::Result { - if !path_read.read_dir_entries { - return Ok(PathFingerprint::Folder(None)); - } - +fn process_directory(path: &std::path::Path) -> anyhow::Result { let mut entries = BTreeMap::new(); for entry in std::fs::read_dir(path)? { let entry = entry?; @@ -472,18 +451,14 @@ fn process_directory( entries.insert(Str::from(name_str.as_ref()), kind); } - Ok(PathFingerprint::Folder(Some(entries))) + Ok(PathFingerprint::Folder(entries)) } /// Process a directory on Unix using nix for efficiency #[cfg(unix)] -fn process_directory_unix(file: &File, path_read: PathRead) -> anyhow::Result { +fn process_directory_unix(file: &File) -> anyhow::Result { use std::os::fd::AsFd; - if !path_read.read_dir_entries { - return Ok(PathFingerprint::Folder(None)); - } - let fd = file.as_fd(); let mut dir = nix::dir::Dir::from_fd(fd.try_clone_to_owned()?)?; @@ -511,7 +486,7 @@ fn process_directory_unix(file: &File, path_read: PathRead) -> anyhow::Result, +} + +impl WorkspaceGitignore { + /// Build a matcher for the workspace root. + /// + /// Collects `.gitignore` at the root plus `.git/info/exclude`. Nested + /// `.gitignore` files inside subdirectories are picked up by the builder as + /// it walks the ones it is given, and a missing file is simply no rules. + /// + /// A malformed pattern is not fatal. Failing the whole task because one line + /// of a `.gitignore` is unparseable would be worse than treating that line as + /// absent, and the fallback direction is safe: fewer ignore rules means more + /// paths look tracked, which means fewer runs are cached. + pub fn open(workspace_root: &AbsolutePath) -> Self { + let mut builder = GitignoreBuilder::new(workspace_root.as_path()); + // Errors here describe individual bad patterns; the partial matcher + // built from the good ones is still worth having. + drop(builder.add(workspace_root.join(".gitignore").as_path())); + drop(builder.add(workspace_root.join(".git/info/exclude").as_path())); + Self { matcher: builder.build().ok() } + } + + /// Whether version control ignores this path. + /// + /// `false` when there is no matcher at all, which is the input-leaning + /// answer. + pub fn is_ignored(&self, path: &RelativePathBuf) -> bool { + let Some(matcher) = &self.matcher else { + return false; + }; + // A directory and a file with the same name match different patterns + // (`dist/` only matches the directory), so the caller's path is checked + // both ways rather than guessing. + matcher.matched_path_or_any_parents(path.as_path(), false).is_ignore() + || matcher.matched_path_or_any_parents(path.as_path(), true).is_ignore() + } +} diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs index 1813051e2..4bf97fe95 100644 --- a/crates/vite_task/src/session/execute/mod.rs +++ b/crates/vite_task/src/session/execute/mod.rs @@ -1,4 +1,8 @@ mod cache_update; +#[cfg(fspy)] +mod classify; +#[cfg(fspy)] +mod gitignore; pub mod fingerprint; pub mod glob; mod hash; diff --git a/crates/vite_task/src/session/execute/tracked_accesses.rs b/crates/vite_task/src/session/execute/tracked_accesses.rs index 6e3596512..fc1c4002c 100644 --- a/crates/vite_task/src/session/execute/tracked_accesses.rs +++ b/crates/vite_task/src/session/execute/tracked_accesses.rs @@ -1,27 +1,25 @@ -//! Normalize raw fspy path accesses into workspace-relative, filtered form. +//! Normalize raw fspy path accesses into workspace-relative, ordered form. +//! +//! Order is preserved because the classification rules depend on it: a task that +//! wrote a path before reading it is observing its own output, while one that +//! read before writing consumed something that existed. Collapsing accesses into +//! per-path read and write sets, as this module used to, throws that away. //! //! User-configured negative globs are NOT applied here. They are applied later, //! separately for reads (input config) and writes (output config), since those //! two configs are independent. #![cfg(fspy)] -use std::collections::hash_map::Entry; - -use fspy::{AccessMode, PathAccessIterable}; -use rustc_hash::FxHashSet; +use fspy::PathAccessIterable; use vite_path::{AbsolutePath, RelativePathBuf}; -use super::fingerprint::PathRead; -use crate::collections::HashMap; +use super::classify::TrackedEvent; -/// Tracked file accesses from fspy, normalized to workspace-relative paths. +/// Tracked file accesses from fspy, normalized to workspace-relative paths and +/// kept in observation order. #[derive(Default, Debug)] pub struct TrackedPathAccesses { - /// Tracked path reads - pub path_reads: HashMap, - - /// Tracked path writes - pub path_writes: FxHashSet, + pub events: Vec, } impl TrackedPathAccesses { @@ -29,7 +27,7 @@ impl TrackedPathAccesses { /// normalizing `..` components. `.git/*` paths are skipped. User-configured /// negatives are applied by the caller (see module docs). pub fn from_raw(raw: &PathAccessIterable, workspace_root: &AbsolutePath) -> Self { - let mut accesses = Self::default(); + let mut events = Vec::new(); for access in raw.iter() { // Strip workspace root and clean `..` components in one pass. // fspy may report paths like `packages/sub-pkg/../shared/dist/output.js`. @@ -44,27 +42,9 @@ impl TrackedPathAccesses { continue; }; - if access.mode.contains(AccessMode::READ) { - accesses - .path_reads - .entry(relative_path.clone()) - .or_insert(PathRead { read_dir_entries: false }); - } - if access.mode.contains(AccessMode::WRITE) { - accesses.path_writes.insert(relative_path.clone()); - } - if access.mode.contains(AccessMode::READ_DIR) { - match accesses.path_reads.entry(relative_path) { - Entry::Occupied(mut occupied) => { - occupied.get_mut().read_dir_entries = true; - } - Entry::Vacant(vacant) => { - vacant.insert(PathRead { read_dir_entries: true }); - } - } - } + events.push(TrackedEvent { path: relative_path, mode: access.mode }); } - accesses + Self { events } } } From 994633f9457c8e13da657ffa2e0ca293f817116c Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:00:50 +0800 Subject: [PATCH 03/21] fix(cache): explain an absent mutation by an ancestor rename Renaming a staging directory takes everything beneath it along, but only the directory itself carries the rename event. Without checking ancestors, every file written under dist.tmp looked like a mutation that vanished unexplained, which blocked caching for the atomic-publish pattern the rename support was added to handle. Co-authored-by: Claude --- .../vite_task/src/session/execute/classify.rs | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index df2ff8a26..9a1a19a78 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -178,6 +178,15 @@ pub fn classify( relocated.iter().map(|(path, history)| (path, history)).collect(); candidates.extend(relocated_entries); + // Directories that were renamed away or removed. Anything that used to live + // beneath one of these is accounted for, so its absence needs no further + // explanation. + let retired_ancestors: Vec<&str> = histories + .iter() + .filter(|(_, history)| history.renamed_away || history.first_deletion.is_some()) + .map(|(path, _)| path.as_str()) + .collect(); + // Every directory this task wrote into, including ancestors. A listing of // one of these enumerated the task's own product, so treating it as a // dependency would make the cache key depend on the output and could never @@ -204,12 +213,21 @@ pub fn classify( let is_directory = metadata.as_ref().is_some_and(std::fs::Metadata::is_dir) || (history.known_directory && !exists); - if history.first_mutation.is_some() { - let cleaned_up = history.first_deletion.is_some() && !exists; - if !exists && !history.renamed_away && !cleaned_up { - // Something changed and then vanished with nothing to explain - // where it went. Whatever produced it is not in the archive, so - // caching this run could restore an incomplete tree. + if history.first_mutation.is_some() && !exists { + // A path that changed and then vanished needs an explanation, or the + // thing that produced it is missing from the archive and a cache hit + // would restore an incomplete tree. Being renamed or removed + // explains it — including by an ancestor, since renaming a staging + // directory takes everything beneath it along and only the directory + // itself carries the rename. + let explained = history.renamed_away + || history.first_deletion.is_some() + || retired_ancestors.iter().any(|retired| { + path.as_str() + .strip_prefix(*retired) + .is_some_and(|tail| tail.starts_with('/')) + }); + if !explained { classification.unexplained_mutation.get_or_insert_with(|| path.clone()); } } From 6d251a7a29883331d2f336f89cfdc8a33cf88c88 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:04:00 +0800 Subject: [PATCH 04/21] test(e2e): O_RDWR without writing now caches touch-file opens O_RDWR and never writes a byte, which leaves the file untouched. Treating write capability as a mutation is the false positive this rule set removes: Biome opens a clean source read-write on every run, and lock files across cargo, rustc and Parcel are opened O_RDWR and only flocked. rw-pkg, which genuinely reads and rewrites a tracked source, still reports that it modified its input. Co-authored-by: Claude --- .../snapshots.toml | 4 ++-- ..._read_write_shows_not_cached_in_summary.md | 6 +++--- .../single_O_RDWR_open_is_not_cached.md | 21 ------------------- ...e_O_RDWR_open_without_writing_is_cached.md | 18 ++++++++++++++++ 4 files changed, 23 insertions(+), 26 deletions(-) delete mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml index e3c51ace1..25bef5d86 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml @@ -22,9 +22,9 @@ cwd = "packages/rw-pkg" steps = [["vt", "run", "-v", "task"]] [[e2e]] -name = "single_O_RDWR_open_is_not_cached" +name = "single_O_RDWR_open_without_writing_is_cached" comment = """ -Opening a single file with `O_RDWR` (e.g. `touch` keeping the file) should count as a read-write overlap and prevent caching, just like separate read+write syscalls. +Opening a file `O_RDWR` and never writing to it leaves the file untouched, so it is a read and not a read-write overlap. Write capability alone must not block caching: formatters such as Biome open a clean source file read-write on every run and only write when there is something to fix, and lock files across cargo, rustc and Parcel are opened `O_RDWR` and only ever flocked. A mutation requires truncation, exclusive creation, or being a rename destination. """ cwd = "packages/touch-pkg" steps = [["vt", "run", "task"], ["vt", "run", "task"]] diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md index 74e809467..649da06e5 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md @@ -13,7 +13,7 @@ hello ~/packages/rw-pkg$ vtt replace-file-content src/data.txt i ! --- -vt run: 0/3 cache hit (0%). @test/touch-pkg#task (and 1 more) not cached because they modified their inputs. (Run `vt run --last-details` for full details) +vt run: 0/3 cache hit (0%). @test/rw-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) ``` ## `vt run -r task` @@ -22,10 +22,10 @@ vt run: 0/3 cache hit (0%). @test/touch-pkg#task (and 1 more) not cached because ~/packages/normal-pkg$ vtt print hello ◉ cache hit, replaying hello -~/packages/touch-pkg$ vtt touch-file src/data.txt +~/packages/touch-pkg$ vtt touch-file src/data.txt ◉ cache hit, replaying ~/packages/rw-pkg$ vtt replace-file-content src/data.txt i ! --- -vt run: 1/3 cache hit (33%). @test/touch-pkg#task (and 1 more) not cached because they modified their inputs. (Run `vt run --last-details` for full details) +vt run: 2/3 cache hit (66%). @test/rw-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) ``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md deleted file mode 100644 index c940d6b28..000000000 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md +++ /dev/null @@ -1,21 +0,0 @@ -# single_O_RDWR_open_is_not_cached - -Opening a single file with `O_RDWR` (e.g. `touch` keeping the file) should count as a read-write overlap and prevent caching, just like separate read+write syscalls. - -## `vt run task` - -``` -~/packages/touch-pkg$ vtt touch-file src/data.txt - ---- -vt run: @test/touch-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) -``` - -## `vt run task` - -``` -~/packages/touch-pkg$ vtt touch-file src/data.txt - ---- -vt run: @test/touch-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) -``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md new file mode 100644 index 000000000..49c4c13be --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md @@ -0,0 +1,18 @@ +# single_O_RDWR_open_without_writing_is_cached + +Opening a file `O_RDWR` and never writing to it leaves the file untouched, so it is a read and not a read-write overlap. Write capability alone must not block caching: formatters such as Biome open a clean source file read-write on every run and only write when there is something to fix, and lock files across cargo, rustc and Parcel are opened `O_RDWR` and only ever flocked. A mutation requires truncation, exclusive creation, or being a rename destination. + +## `vt run task` + +``` +~/packages/touch-pkg$ vtt touch-file src/data.txt +``` + +## `vt run task` + +``` +~/packages/touch-pkg$ vtt touch-file src/data.txt ◉ cache hit, replaying + +--- +vt run: cache hit. +``` From 74b23dffca184d59dd62691eb4ff6c27e4380d23 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:07:11 +0800 Subject: [PATCH 05/21] test(e2e): cover atomic publish, output cleanup and managed state Three cases for the auto-tracking rules, each asserting the outcome that matters rather than just a cache hit. atomic-pkg stages under dist.tmp and renames the directory into place, then removes dist and runs again to prove the outputs came back from the archive. clean-pkg empties its output directory every run, which is what makes a build tool stat and list its own product. state-pkg reads and rewrites a gitignored file, the cache half of a command like eslint --fix --cache. The work happens inside one vtt process because a compound 'a && b' command is cached as separate tasks, which would hide the relationship under test. Co-authored-by: Claude --- crates/vite_task_bin/src/vtt/main.rs | 6 ++- crates/vite_task_bin/src/vtt/publish_dir.rs | 48 +++++++++++++++++++ crates/vite_task_bin/src/vtt/rename.rs | 15 ++++++ .../fixtures/auto_output_tracking/.gitignore | 3 ++ .../auto_output_tracking/package.json | 1 + .../packages/atomic-pkg/package.json | 1 + .../packages/atomic-pkg/src/data.txt | 1 + .../packages/atomic-pkg/vite-task.json | 7 +++ .../packages/clean-pkg/package.json | 1 + .../packages/clean-pkg/src/data.txt | 1 + .../packages/clean-pkg/vite-task.json | 7 +++ .../packages/state-pkg/package.json | 1 + .../packages/state-pkg/src/data.txt | 1 + .../packages/state-pkg/vite-task.json | 7 +++ .../auto_output_tracking/pnpm-workspace.yaml | 2 + .../auto_output_tracking/snapshots.toml | 39 +++++++++++++++ ...irectory_publish_is_cached_and_restored.md | 38 +++++++++++++++ ..._state_read_then_rewritten_is_an_output.md | 35 ++++++++++++++ .../output_directory_cleanup_still_caches.md | 38 +++++++++++++++ .../auto_output_tracking/vite-task.json | 1 + 20 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 crates/vite_task_bin/src/vtt/publish_dir.rs create mode 100644 crates/vite_task_bin/src/vtt/rename.rs create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json diff --git a/crates/vite_task_bin/src/vtt/main.rs b/crates/vite_task_bin/src/vtt/main.rs index 9e3c7f6fb..e295c510a 100644 --- a/crates/vite_task_bin/src/vtt/main.rs +++ b/crates/vite_task_bin/src/vtt/main.rs @@ -22,6 +22,8 @@ mod print_env; mod print_file; mod read_stdin; mod replace_file_content; +mod publish_dir; +mod rename; mod rm; #[cfg(target_os = "linux")] mod small_dev_shm; @@ -35,7 +37,7 @@ fn main() { if args.len() < 2 { eprintln!("Usage: vtt [args...]"); eprintln!( - "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, read-stdin, replace-file-content, rm, small_dev_shm, stat-file, stat_long_filename, touch-file, write-file" + "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, publish-dir, read-stdin, rename, replace-file-content, rm, small_dev_shm, stat-file, stat_long_filename, touch-file, write-file" ); std::process::exit(1); } @@ -76,6 +78,8 @@ fn main() { Ok(()) } "stat_long_filename" => stat_long_filename::run(&args[2..]), + "publish-dir" => publish_dir::run(&args[2..]), + "rename" => rename::run(&args[2..]), "touch-file" => touch_file::run(&args[2..]), "write-file" => write_file::run(&args[2..]), other => { diff --git a/crates/vite_task_bin/src/vtt/publish_dir.rs b/crates/vite_task_bin/src/vtt/publish_dir.rs new file mode 100644 index 000000000..79ad8a6fc --- /dev/null +++ b/crates/vite_task_bin/src/vtt/publish_dir.rs @@ -0,0 +1,48 @@ +//! Publish an output directory the way real build tools do, in one process. +//! +//! Both modes exist because they stress different parts of auto-output tracking, +//! and both must happen inside a single task: a compound `a && b` command is +//! cached as separate tasks, so splitting the steps would hide the very +//! relationship under test. +//! +//! `atomic` stages the output under a temporary directory and renames that +//! directory into place. Every write lands on the staging path and only the +//! directory carries the rename, so a tracker that ignores directory renames +//! collects nothing. +//! +//! `rebuild` empties the output directory first, which is what makes a build +//! tool stat and list its own output. Those reads must not become inputs. + +use std::path::Path; + +pub fn run(args: &[String]) -> Result<(), Box> { + let [mode, source, directory] = args else { + return Err("Usage: vtt publish-dir ".into()); + }; + let directory = Path::new(directory); + let contents = std::fs::read_to_string(source)?; + + match mode.as_str() { + "atomic" => { + let staging = directory.with_extension("tmp"); + if staging.exists() { + std::fs::remove_dir_all(&staging)?; + } + std::fs::create_dir_all(&staging)?; + std::fs::write(staging.join("out.txt"), &contents)?; + if directory.exists() { + std::fs::remove_dir_all(directory)?; + } + std::fs::rename(&staging, directory)?; + } + "rebuild" => { + if directory.exists() { + std::fs::remove_dir_all(directory)?; + } + std::fs::create_dir_all(directory)?; + std::fs::write(directory.join("out.txt"), &contents)?; + } + other => return Err(format!("unknown mode: {other}").into()), + } + Ok(()) +} diff --git a/crates/vite_task_bin/src/vtt/rename.rs b/crates/vite_task_bin/src/vtt/rename.rs new file mode 100644 index 000000000..a333cb052 --- /dev/null +++ b/crates/vite_task_bin/src/vtt/rename.rs @@ -0,0 +1,15 @@ +//! Rename a file or a whole directory, the way tools publish results atomically. +//! +//! Build tools commonly stage output under a temporary name and rename it into +//! place so readers never see a half-written tree. Renaming a *directory* is the +//! interesting case for tracking: every write lands on the staging path, and only +//! the directory itself carries the rename, so a tracker that ignores directory +//! renames loses every output. + +pub fn run(args: &[String]) -> Result<(), Box> { + if args.len() != 2 { + return Err("Usage: vtt rename ".into()); + } + std::fs::rename(&args[0], &args[1])?; + Ok(()) +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore new file mode 100644 index 000000000..8b69f3a68 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore @@ -0,0 +1,3 @@ +node_modules/ +dist/ +.cache/ diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json new file mode 100644 index 000000000..417e4e013 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json @@ -0,0 +1 @@ +{ "name": "auto-output-tracking", "private": true, "version": "0.0.0" } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json new file mode 100644 index 000000000..c8f1d2186 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json @@ -0,0 +1 @@ +{ "name": "@test/atomic-pkg", "version": "0.0.0" } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt new file mode 100644 index 000000000..63695f771 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt @@ -0,0 +1 @@ +atomic diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json new file mode 100644 index 000000000..91d5fd62a --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir atomic src/data.txt dist" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json new file mode 100644 index 000000000..b55cbfa65 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json @@ -0,0 +1 @@ +{ "name": "@test/clean-pkg", "version": "0.0.0" } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt new file mode 100644 index 000000000..831263020 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt @@ -0,0 +1 @@ +clean diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json new file mode 100644 index 000000000..ceac42463 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir rebuild src/data.txt dist" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json new file mode 100644 index 000000000..7f1a13933 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json @@ -0,0 +1 @@ +{ "name": "@test/state-pkg", "version": "0.0.0" } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt new file mode 100644 index 000000000..ff72b5c73 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt @@ -0,0 +1 @@ +state diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json new file mode 100644 index 000000000..aaef5ca78 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt mkdir .cache && vtt cp src/data.txt .cache/state.txt && vtt replace-file-content .cache/state.txt e E" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml new file mode 100644 index 000000000..924b55f42 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - packages/* diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml new file mode 100644 index 000000000..001ba1485 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml @@ -0,0 +1,39 @@ +[[e2e]] +name = "atomic_directory_publish_is_cached_and_restored" +comment = """ +A task that stages output under `dist.tmp` and renames the directory into place must still have its outputs collected. Every write lands on the staging path and only the directory itself carries the rename, so a tracker that ignores directory renames archives nothing and a later cache hit restores an incomplete tree. The third run removes `dist` first to prove the outputs really came back from the archive rather than from the previous run leaving them behind. +""" +cwd = "packages/atomic-pkg" +steps = [ + ["vt", "run", "task"], + ["vt", "run", "task"], + ["vtt", "rm", "-rf", "dist"], + ["vt", "run", "task"], + ["vtt", "print-file", "dist/out.txt"], +] + +[[e2e]] +name = "output_directory_cleanup_still_caches" +comment = """ +Build tools empty their output directory before writing to it, which means they stat and list it. Those reads must not make the directory an input: its contents are the task's own product, and a fingerprint of them can never match on a clean checkout where the directory does not exist yet. +""" +cwd = "packages/clean-pkg" +steps = [ + ["vt", "run", "task"], + ["vt", "run", "task"], + ["vtt", "rm", "-rf", "dist"], + ["vt", "run", "task"], + ["vtt", "print-file", "dist/out.txt"], +] + +[[e2e]] +name = "gitignored_state_read_then_rewritten_is_an_output" +comment = """ +A path a task reads and then rewrites is either a source it fixed up or state it manages, and access mechanics cannot tell those apart. Version control can: this file is gitignored, so it is derived state and belongs in the archive. Compare `input_read_write_not_cached`, where the same access pattern on a tracked source blocks caching instead. +""" +cwd = "packages/state-pkg" +steps = [ + ["vt", "run", "task"], + ["vt", "run", "task"], + ["vtt", "print-file", ".cache/state.txt"], +] diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md new file mode 100644 index 000000000..09e218965 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md @@ -0,0 +1,38 @@ +# atomic_directory_publish_is_cached_and_restored + +A task that stages output under `dist.tmp` and renames the directory into place must still have its outputs collected. Every write lands on the staging path and only the directory itself carries the rename, so a tracker that ignores directory renames archives nothing and a later cache hit restores an incomplete tree. The third run removes `dist` first to prove the outputs really came back from the archive rather than from the previous run leaving them behind. + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist +``` + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt rm -rf dist` + +``` +``` + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt print-file dist/out.txt` + +``` +atomic +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md new file mode 100644 index 000000000..e7553c278 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md @@ -0,0 +1,35 @@ +# gitignored_state_read_then_rewritten_is_an_output + +A path a task reads and then rewrites is either a source it fixed up or state it manages, and access mechanics cannot tell those apart. Version control can: this file is gitignored, so it is derived state and belongs in the archive. Compare `input_read_write_not_cached`, where the same access pattern on a tracked source blocks caching instead. + +## `vt run task` + +``` +~/packages/state-pkg$ vtt mkdir .cache + +~/packages/state-pkg$ vtt cp src/data.txt .cache/state.txt + +~/packages/state-pkg$ vtt replace-file-content .cache/state.txt e E + +--- +vt run: 0/3 cache hit (0%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/state-pkg$ vtt mkdir .cache ◉ cache hit, replaying + +~/packages/state-pkg$ vtt cp src/data.txt .cache/state.txt ◉ cache hit, replaying + +~/packages/state-pkg$ vtt replace-file-content .cache/state.txt e E ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` + +## `vtt print-file .cache/state.txt` + +``` +statE +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md new file mode 100644 index 000000000..cc0836622 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md @@ -0,0 +1,38 @@ +# output_directory_cleanup_still_caches + +Build tools empty their output directory before writing to it, which means they stat and list it. Those reads must not make the directory an input: its contents are the task's own product, and a fingerprint of them can never match on a clean checkout where the directory does not exist yet. + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist +``` + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt rm -rf dist` + +``` +``` + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt print-file dist/out.txt` + +``` +clean +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json new file mode 100644 index 000000000..d0fd62ced --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json @@ -0,0 +1 @@ +{ "cache": true } From 667b75a8c94c8a9ad852d74d2915852e2a8e3897 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:11:32 +0800 Subject: [PATCH 06/21] refactor(cache): split classify into folding and subtree phases Satisfies clippy line limits and makes the two phases legible: folding the event stream into per-path histories, then deriving the directory subtrees the task wrote into. Co-authored-by: Claude --- .../src/interceptions/mutate.rs | 3 +- .../vite_task/src/session/execute/classify.rs | 68 ++++++++++++------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/crates/fspy_preload_unix/src/interceptions/mutate.rs b/crates/fspy_preload_unix/src/interceptions/mutate.rs index c273b18c3..615ca0a53 100644 --- a/crates/fspy_preload_unix/src/interceptions/mutate.rs +++ b/crates/fspy_preload_unix/src/interceptions/mutate.rs @@ -32,6 +32,7 @@ use crate::{ /// inspecting the rename's result: it describes an argument, and it happens /// before the call while the source is still there. unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { + // SAFETY: an all-zero stat is a valid initial value; fstatat overwrites it let mut stat_buf: libc::stat = unsafe { core::mem::zeroed() }; // SAFETY: path is a valid C string pointer and stat_buf is a valid, owned stat struct let result = unsafe { libc::fstatat(dirfd, path, &raw mut stat_buf, libc::AT_SYMLINK_NOFOLLOW) }; @@ -106,7 +107,7 @@ unsafe extern "C" fn renameatx_np( unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; // RENAME_SWAP mutates both sides, so the source is replaced too rather than // simply retired. - if flags & u32::try_from(libc::RENAME_SWAP).unwrap_or(0) != 0 { + if flags & libc::RENAME_SWAP != 0 { // SAFETY: old_path is a valid C string pointer provided by the caller unsafe { handle_open( diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index 9a1a19a78..9c8e21464 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -91,7 +91,7 @@ struct PathHistory { } impl PathHistory { - fn observe(&mut self, index: usize, mode: AccessMode) { + const fn observe(&mut self, index: usize, mode: AccessMode) { if mode.contains(AccessMode::IS_DIR) { self.known_directory = true; } @@ -120,11 +120,11 @@ impl PathHistory { /// /// `already_fingerprinted` reports paths the caller hashed before the run, which /// do not need fingerprinting again. -pub fn classify( +/// Fold the event stream into per-path histories, and collect the directory +/// renames that need writes re-attributed. +fn fold_events( events: &[TrackedEvent], - context: &ClassifyContext<'_>, - already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, -) -> Classification { +) -> (FxHashMap<&RelativePathBuf, PathHistory>, Vec<(&RelativePathBuf, &RelativePathBuf)>) { let mut histories: FxHashMap<&RelativePathBuf, PathHistory> = FxHashMap::default(); // Directory renames, newest last. A build that stages into `dist.tmp` and // swaps it into place records every write against the staging path, so those @@ -135,7 +135,7 @@ pub fn classify( for (index, event) in events.iter().enumerate() { histories.entry(&event.path).or_default().observe(index, event.mode); - // The preload reports a rename as source then destination, adjacent. + // A rename is reported as source then destination, adjacent. if event.mode.contains(AccessMode::RENAME_FROM) { last_rename_source = Some(&event.path); } else if event.mode.contains(AccessMode::RENAME_TO) { @@ -147,7 +147,40 @@ pub fn classify( last_rename_source = None; } } + (histories, directory_renames) +} + +/// Every directory the task wrote into, including ancestors. +/// +/// A listing of one of these enumerated the task's own product, so treating it +/// as a dependency would make the cache key depend on the output and could never +/// match on a clean checkout where the directory does not exist yet. +fn mutated_subtrees<'a>( + candidates: &[(&'a RelativePathBuf, &'a PathHistory)], +) -> FxHashSet<&'a str> { + let mut subtrees: FxHashSet<&str> = FxHashSet::default(); + for (path, history) in candidates { + if history.first_mutation.is_none() { + continue; + } + let mut remaining = path.as_str(); + while let Some(separator) = remaining.rfind('/') { + remaining = &remaining[..separator]; + if !subtrees.insert(remaining) { + // An ancestor already recorded, so the rest are too. + break; + } + } + } + subtrees +} +pub fn classify( + events: &[TrackedEvent], + context: &ClassifyContext<'_>, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) -> Classification { + let (histories, directory_renames) = fold_events(events); let mut classification = Classification::default(); // Re-attribute mutations recorded beneath a renamed directory. @@ -175,7 +208,7 @@ pub fn classify( let mut candidates: Vec<(&RelativePathBuf, &PathHistory)> = histories.iter().map(|(path, history)| (*path, history)).collect(); let relocated_entries: Vec<(&RelativePathBuf, &PathHistory)> = - relocated.iter().map(|(path, history)| (path, history)).collect(); + relocated.iter().collect(); candidates.extend(relocated_entries); // Directories that were renamed away or removed. Anything that used to live @@ -187,24 +220,7 @@ pub fn classify( .map(|(path, _)| path.as_str()) .collect(); - // Every directory this task wrote into, including ancestors. A listing of - // one of these enumerated the task's own product, so treating it as a - // dependency would make the cache key depend on the output and could never - // match on a clean checkout, where the directory does not exist yet. - let mut mutated_subtrees: FxHashSet<&str> = FxHashSet::default(); - for (path, history) in &candidates { - if history.first_mutation.is_none() { - continue; - } - let mut remaining = path.as_str(); - while let Some(separator) = remaining.rfind('/') { - remaining = &remaining[..separator]; - if !mutated_subtrees.insert(remaining) { - // An ancestor already recorded, so the rest are too. - break; - } - } - } + let mutated_subtrees = mutated_subtrees(&candidates); for (path, history) in candidates { let absolute = context.workspace_root.join(path); @@ -271,7 +287,7 @@ pub fn classify( /// probe of the task's own output directory. A listing is a real dependency, /// unless the task wrote into that directory, in which case the entries it saw /// are its own product. -fn is_input_candidate( +const fn is_input_candidate( history: &PathHistory, is_directory: bool, wrote_into_subtree: bool, From b06ab58050b6c47c4d630c3dde771174d2260b03 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:49:21 +0800 Subject: [PATCH 07/21] docs: record local verification against the emdash workload Co-authored-by: Claude --- DECISIONS.md | 46 +++++++++++++++++++ .../src/session/execute/cache_update.rs | 23 ++++++++++ .../vite_task/src/session/execute/classify.rs | 25 ++++++++++ .../src/session/execute/gitignore.rs | 31 +++++++++++++ 4 files changed, 125 insertions(+) diff --git a/DECISIONS.md b/DECISIONS.md index 7f0630893..7c860dbcc 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -166,3 +166,49 @@ over-collects its destination, which rule 7 permits. **Not an outcome.** Checking whether a path exists at archive time is a filesystem query at decision time, not a syscall outcome, and it stays. It is also what filters the over-collection above. + +## D10 — Verified against the real emdash workload before touching Windows + +Reordered on request to de-risk the chain: prove the goal on macOS with a local +`[patch]` override before investing in the Windows backend. + +**Setup.** A vite-plus worktree at +`~/.local/share/opencode/worktree/vite-plus-autotrack` with the local-development +patch section enabled, pointing every vite-task crate at this worktree. + +**Two traps found while doing it, both worth remembering.** + +The `vp` binary does not link `vite_task` at all — only `packages/cli/binding`, +the napi addon, does. So `vp run` is JavaScript calling a native addon, and every +early emdash run measured released vite-plus 0.2.2 rather than this branch. +Swapping a locally built `.node` into emdash's store does not work either: the +addon needs `--features rolldown`, and a 0.2.6 addon against a 0.2.2 JS CLI +loads but produces no output. Verification therefore ran through `vt`, vite-task's +own CLI, which links the crates directly. + +Separately, three artifacts must be rebuilt for a change to take effect, and each +one silently reported stale results when missed: the preload dylib consumed by +`fspy --example cli`, the `vt` binary, and the napi addon. + +**Result on `@emdash-cms/auth`, a plain `tsdown` build, with no manual globs:** + +- run 1 cold: builds, 28 outputs collected, 3074 inputs +- run 2 unchanged: cache hit +- run 3 after `rm -rf dist`: cache hit **and** all 28 files restored +- restored tree is byte-for-byte identical to a fresh build, verified by shasum +- **zero paths under the package's own `dist/` appear in the input set** + +That last point is the failure mode that matters: if outputs were fingerprinted +as inputs, run 3 would miss forever on a clean checkout. + +## D11 — emdash's full build is blocked by its own environment, not by tracking + +`@emdash-cms/registry-lexicons#build` runs `pnpm run build:lexicons` and fails +under both this branch and released vite-plus. Released vite-plus reports the +real cause: `ERR_PNPM_VERIFY_DEPS_BEFORE_RUN`, the workspace structure changed +since the last install. Under `vt` the same failure surfaces less legibly, as a +pnpm SEA assertion (`(magic) == (kMagic)`) because pnpm is a single-executable +binary re-reading its own embedded blob. + +Not a tracking defect. emdash needs `pnpm install` before the full build can be +measured. diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index 6b36dfea0..a2926a4ec 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -255,6 +255,9 @@ fn observe_fspy( .cloned() .collect(); + let modified_for_debug = classification.modified_input.clone(); + let unexplained_for_debug = classification.unexplained_mutation.clone(); + // A modified input only blocks caching if it is still tracked as an // input after exclusions; excluding it is the user saying they know. let read_write_overlap = classification @@ -266,6 +269,26 @@ fn observe_fspy( .filter(|path| !excluded_from_outputs(path)) }); + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] + { + eprintln!( + "[tracking] result inputs={} outputs={} modified={:?} unexplained={:?} \ + overlap={:?} infer_in={infer_inputs} infer_out={infer_outputs}", + classification.inputs.len(), + classification.outputs.len(), + modified_for_debug, + unexplained_for_debug, + read_write_overlap, + ); + let leaked: Vec<&str> = path_reads + .iter() + .map(|path| path.as_str()) + .filter(|path| path.contains("/dist/") || path.starts_with("dist/")) + .collect(); + eprintln!("[tracking] output paths fingerprinted as inputs: {leaked:?}"); + } + } TrackingOutcome { path_reads, path_writes, read_write_overlap } }) } diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index 9c8e21464..0fb2de942 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -244,6 +244,19 @@ pub fn classify( .is_some_and(|tail| tail.starts_with('/')) }); if !explained { + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] + { + eprintln!( + "[tracking] unexplained {path} renamed_away={} deleted={:?} \ + mutation={:?} read={:?}", + history.renamed_away, + history.first_deletion, + history.first_mutation, + history.first_content_read, + ); + } + } classification.unexplained_mutation.get_or_insert_with(|| path.clone()); } } @@ -261,6 +274,18 @@ pub fn classify( } (true, true) => { let write_first = history.first_mutation < history.first_content_read; + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] + { + eprintln!( + "[tracking] overlap {path} write_first={write_first} \ + gitignored={} read={:?} mutation={:?}", + (context.is_gitignored)(path), + history.first_content_read, + history.first_mutation, + ); + } + } if write_first || (context.is_gitignored)(path) { if exists { classification.outputs.insert(path.clone()); diff --git a/crates/vite_task/src/session/execute/gitignore.rs b/crates/vite_task/src/session/execute/gitignore.rs index 739621b9a..64d8ceddf 100644 --- a/crates/vite_task/src/session/execute/gitignore.rs +++ b/crates/vite_task/src/session/execute/gitignore.rs @@ -62,3 +62,34 @@ impl WorkspaceGitignore { || matcher.matched_path_or_any_parents(path.as_path(), true).is_ignore() } } + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + use vite_path::AbsolutePathBuf; + + use super::*; + + /// A root `dist/` rule must ignore a nested build output. This is the shape + /// every real monorepo has, and getting it wrong turns every package's + /// output into a modified input. + #[test] + fn root_directory_rule_ignores_nested_output() { + let temp = TempDir::new().unwrap(); + let root = AbsolutePathBuf::new(temp.path().to_path_buf()).unwrap(); + std::fs::write(root.join(".gitignore").as_path(), "dist/\nnode_modules\n").unwrap(); + std::fs::create_dir_all(root.join("packages/auth/dist").as_path()).unwrap(); + std::fs::write(root.join("packages/auth/dist/index.mjs").as_path(), "x").unwrap(); + + let gitignore = WorkspaceGitignore::open(&root); + + assert!( + gitignore.is_ignored(&RelativePathBuf::new("packages/auth/dist/index.mjs").unwrap()), + "a root `dist/` rule must cover nested package output" + ); + assert!( + !gitignore.is_ignored(&RelativePathBuf::new("packages/auth/src/index.ts").unwrap()), + "tracked sources must not be reported as ignored" + ); + } +} From 24d25c975da50d62856b9f7c051d2c31e0eb4231 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 21:52:29 +0800 Subject: [PATCH 08/21] docs: record the pre-existing pnpm SEA failure under fspy Co-authored-by: Claude --- DECISIONS.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/DECISIONS.md b/DECISIONS.md index 7c860dbcc..b82132d4d 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -212,3 +212,20 @@ binary re-reading its own embedded blob. Not a tracking defect. emdash needs `pnpm install` before the full build can be measured. + +## D12 — pnpm's single-executable binary fails under fspy; pre-existing + +`@emdash-cms/registry-lexicons#build` shells out to `pnpm run build:lexicons` and +aborts with `Assertion failed: (magic) == (kMagic)` inside +`node::sea::FindSingleExecutableResource`. pnpm 11.9.0 ships as a Node +single-executable application, so it re-reads its own binary to find an embedded +blob, and that read does not survive tracing. + +**Verified pre-existing**, not caused by this branch: a `vt` built from the base +commit `b3ebf564` reproduces the identical assertion. Released vite-plus does not +hit it because it supplies a managed Node and pnpm rather than the mise shim. + +Left alone. It is orthogonal to auto-tracking, blocks only the one package whose +build re-enters pnpm, and fixing it means changing how fspy handles a process +reading its own executable. Verification of the tracking rules therefore excludes +that package. From cbb192a5c346477feffc64c4f4da801dea466a82 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:18:39 +0800 Subject: [PATCH 09/21] fix(cache): treat a task's own derived output tree as not an input emdash's admin build missed on every run. Its build is a compound command whose segments are separate tasks, and the tsdown segment reads dist/locales//messages.mjs while a later segment rewrites those files, so the fingerprint could never settle. Neither existing rule could reject it. The task did not write into dist/locales itself, a sibling did, so the wrote-into-subtree rule stays quiet. Rejecting purely on gitignore is unsafe because a package reading node_modules//dist is also reading ignored derived content, and that must remain an input or changing a dependency stops invalidating its consumers. A read now counts as the task's own derived state only when version control calls it derived and it sits under a directory this same task wrote into. Directory listings apply the same test. emdash's 23 package builds now cache 23/23 with no manual globs, and deleting every dist restores all 1358 files byte-for-byte. Co-authored-by: Claude --- DECISIONS.md | 43 +++++ .../src/session/execute/cache_update.rs | 6 - .../vite_task/src/session/execute/classify.rs | 167 +++++++++++++----- 3 files changed, 165 insertions(+), 51 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index b82132d4d..760b11432 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -229,3 +229,46 @@ Left alone. It is orthogonal to auto-tracking, blocks only the one package whose build re-enters pnpm, and fixing it means changing how fspy handles a process reading its own executable. Verification of the tracking rules therefore excludes that package. + +## D13 — A task's own derived output tree is not an input + +Found by running emdash's full build, not by reasoning. With the rules as first +written, 22 of 23 tasks cached and `@emdash-cms/admin#build`'s `tsdown` segment +missed forever with `'messages.mjs' added in 'packages/admin/dist/locales/fa'`. + +**Cause.** admin's build is a compound command, and each `&&` segment is a +separate cached task: + +``` +node --run locale:compile && tsdown && node --run locale:copy && npx tailwindcss +``` + +`tsdown` *reads* `dist/locales//messages.mjs`, and the later +`locale:copy` segment rewrites those files. So tsdown fingerprints content that a +sibling task changes afterwards, and the fingerprint can never settle. + +**Why the existing rules could not catch it.** The "listed a directory it wrote +into" rule does not fire, because tsdown did not write there — a sibling did. +Rejecting on gitignore alone is not available either: a package reading +`node_modules//dist/index.mjs` is also reading ignored, derived content, and +that must stay a real input or changing a dependency would stop invalidating its +consumers. That is the central monorepo relationship. + +**Rule.** A read path is the task's own derived state when **both** hold: version +control calls it derived, **and** it sits under a directory this same task wrote +into. Requiring both distinguishes the two cases exactly — admin writes +`dist/index.mjs`, so `packages/admin/dist` is one of its own write subtrees and +everything ignored beneath it is its own output tree, while no package writes into +`node_modules//dist`, so dependency outputs stay inputs. + +Directory listings follow the same principle: a listing is rejected if the task +wrote into that directory or if version control calls the directory derived. + +**Result on emdash, zero manual globs:** + +- 23/23 cache hit on the second run, stable across repeats +- after deleting every `packages/*/dist`, 22/23 hit and all 1358 files restored + **byte-for-byte identical** +- the single miss is correct: deleting `packages/core/dist` also empties + `node_modules/emdash/dist`, which is a genuine input of another task, so it + rebuilt diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index a2926a4ec..046511993 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -281,12 +281,6 @@ fn observe_fspy( unexplained_for_debug, read_write_overlap, ); - let leaked: Vec<&str> = path_reads - .iter() - .map(|path| path.as_str()) - .filter(|path| path.contains("/dist/") || path.starts_with("dist/")) - .collect(); - eprintln!("[tracking] output paths fingerprinted as inputs: {leaked:?}"); } } TrackingOutcome { path_reads, path_writes, read_write_overlap } diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index 0fb2de942..f72597047 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -175,28 +175,47 @@ fn mutated_subtrees<'a>( subtrees } -pub fn classify( - events: &[TrackedEvent], - context: &ClassifyContext<'_>, - already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, -) -> Classification { - let (histories, directory_renames) = fold_events(events); - let mut classification = Classification::default(); +/// Whether a mutated path that is now gone can be accounted for. +/// +/// Without an explanation the thing that produced it is missing from the archive +/// and a cache hit would restore an incomplete tree. Being renamed or removed +/// explains it, including by an ancestor: renaming a staging directory takes +/// everything beneath it along, and only the directory itself carries the rename. +fn absence_is_explained( + path: &RelativePathBuf, + history: &PathHistory, + retired_ancestors: &[&str], +) -> bool { + history.renamed_away + || history.first_deletion.is_some() + || retired_ancestors.iter().any(|retired| { + path.as_str().strip_prefix(*retired).is_some_and(|tail| tail.starts_with('/')) + }) +} - // Re-attribute mutations recorded beneath a renamed directory. +/// Re-attribute mutations recorded beneath a renamed directory to the published +/// location. +/// +/// A build that stages into `dist.tmp` and swaps it into place records every +/// write against the staging path. Only the directory carries the rename, so +/// without this the real outputs are never collected and a cache hit restores an +/// empty tree. +fn relocate_directory_renames( + histories: &FxHashMap<&RelativePathBuf, PathHistory>, + directory_renames: &[(&RelativePathBuf, &RelativePathBuf)], +) -> FxHashMap { let mut relocated: FxHashMap = FxHashMap::default(); - for (source, destination) in &directory_renames { + for (source, destination) in directory_renames { let source_prefix = source.as_str(); - for (path, history) in &histories { - let Some(tail) = path.as_str().strip_prefix(source_prefix) else { - continue; - }; - let Some(tail) = tail.strip_prefix('/') else { - continue; - }; + for (path, history) in histories { if history.first_mutation.is_none() { continue; } + let Some(tail) = + path.as_str().strip_prefix(source_prefix).and_then(|tail| tail.strip_prefix('/')) + else { + continue; + }; let Ok(moved) = RelativePathBuf::new(vite_str::format!("{destination}/{tail}").as_str()) else { continue; @@ -204,6 +223,18 @@ pub fn classify( relocated.entry(moved).or_default().first_mutation = history.first_mutation; } } + relocated +} + +pub fn classify( + events: &[TrackedEvent], + context: &ClassifyContext<'_>, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) -> Classification { + let (histories, directory_renames) = fold_events(events); + let mut classification = Classification::default(); + + let relocated = relocate_directory_renames(&histories, &directory_renames); let mut candidates: Vec<(&RelativePathBuf, &PathHistory)> = histories.iter().map(|(path, history)| (*path, history)).collect(); @@ -229,21 +260,11 @@ pub fn classify( let is_directory = metadata.as_ref().is_some_and(std::fs::Metadata::is_dir) || (history.known_directory && !exists); - if history.first_mutation.is_some() && !exists { - // A path that changed and then vanished needs an explanation, or the - // thing that produced it is missing from the archive and a cache hit - // would restore an incomplete tree. Being renamed or removed - // explains it — including by an ancestor, since renaming a staging - // directory takes everything beneath it along and only the directory - // itself carries the rename. - let explained = history.renamed_away - || history.first_deletion.is_some() - || retired_ancestors.iter().any(|retired| { - path.as_str() - .strip_prefix(*retired) - .is_some_and(|tail| tail.starts_with('/')) - }); - if !explained { + if history.first_mutation.is_some() + && !exists + && !absence_is_explained(path, history, &retired_ancestors) + { + { if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] { @@ -262,9 +283,16 @@ pub fn classify( } let wrote_into_subtree = is_directory && mutated_subtrees.contains(path.as_str()); + let own_derived_state = is_own_derived_state( + path, + is_directory, + wrote_into_subtree, + context, + &mutated_subtrees, + ); let mutated = history.first_mutation.is_some() && context.infer_outputs; let consumed = - is_input_candidate(history, is_directory, wrote_into_subtree) && context.infer_inputs; + is_input_candidate(history, is_directory, own_derived_state) && context.infer_inputs; match (mutated, consumed) { (true, false) => { @@ -279,10 +307,8 @@ pub fn classify( { eprintln!( "[tracking] overlap {path} write_first={write_first} \ - gitignored={} read={:?} mutation={:?}", + gitignored={}", (context.is_gitignored)(path), - history.first_content_read, - history.first_mutation, ); } } @@ -307,28 +333,79 @@ pub fn classify( /// Whether a path's reads make it a dependency. /// -/// Directories are the interesting case. A bare stat of one carries only -/// existence and type, which cannot detect a content change and is routinely a -/// probe of the task's own output directory. A listing is a real dependency, -/// unless the task wrote into that directory, in which case the entries it saw -/// are its own product. +/// Directories are the interesting case, and three kinds of read are rejected. +/// +/// A bare stat carries only existence and type. It cannot detect a content +/// change and is routinely a probe of the task's own output directory, where a +/// stored fingerprint could never match on a clean checkout. +/// +/// A listing of a directory the task wrote into enumerated the task's own +/// product. +/// +/// A listing of a *gitignored* directory enumerated derived state. Version +/// control already says this content is not source, so depending on the set of +/// names in it is not a source dependency — and that set moves for reasons this +/// task does not control. In emdash, `tsdown` lists `dist/locales` while a +/// sibling task writes into it, so without this the tsdown task invalidates +/// every time a locale is added, even though its own inputs never changed. const fn is_input_candidate( history: &PathHistory, is_directory: bool, - wrote_into_subtree: bool, + own_derived_state: bool, ) -> bool { if history.first_content_read.is_none() { return false; } + if own_derived_state { + return false; + } if !is_directory { return true; } - if history.first_listing.is_none() { - // Stat only. Existence alone is not worth a fingerprint, and a stored - // one can never match on a clean checkout. + // A bare stat of a directory carries only existence and type. It cannot + // detect a content change and is routinely a probe of the task's own output + // directory, where a stored fingerprint could never match on a clean + // checkout. + history.first_listing.is_some() +} + +/// Whether a read path is part of the task's own derived output tree. +/// +/// Two signals have to agree. Version control must call the path derived, and it +/// must sit under a directory this same task wrote into. Requiring both is what +/// keeps the ordinary monorepo dependency intact: a package reading +/// `node_modules//dist/index.mjs` is reading ignored, derived content, but +/// the reader never writes there, so it stays a real input and a change to the +/// dependency still invalidates the consumer. +/// +/// What it does reject is a task reading inside its own output directory. +/// emdash's admin package is the case that forced this: `tsdown` reads +/// `dist/locales//messages.mjs` while a later step in the same build +/// rewrites those files, so fingerprinting them can never settle and the task +/// misses on every run. +fn is_own_derived_state( + path: &RelativePathBuf, + is_directory: bool, + wrote_into_subtree: bool, + context: &ClassifyContext<'_>, + mutated_subtrees: &FxHashSet<&str>, +) -> bool { + if is_directory { + // The listing enumerated names this task produced, or names version + // control considers derived and therefore outside the source graph. + return wrote_into_subtree || (context.is_gitignored)(path); + } + if !(context.is_gitignored)(path) { return false; } - !wrote_into_subtree + let mut remaining = path.as_str(); + while let Some(separator) = remaining.rfind('/') { + remaining = &remaining[..separator]; + if mutated_subtrees.contains(remaining) { + return true; + } + } + false } fn record_input( From 2ca7183545383ab1cd0280522caaf39b97ad3173 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:20:04 +0800 Subject: [PATCH 10/21] test(e2e): lock in the sibling-task read of an own output tree Reproduces emdash's admin build: a compound command whose middle segment reads a file inside its own output directory while a later segment writes there. Each && segment is a separate cached task, so this is the shape that used to miss on every run. Co-authored-by: Claude --- .../packages/sibling-pkg/package.json | 1 + .../packages/sibling-pkg/src/data.txt | 1 + .../packages/sibling-pkg/vite-task.json | 7 ++++ .../auto_output_tracking/snapshots.toml | 12 ++++++ ...ing_inside_own_output_tree_still_caches.md | 42 +++++++++++++++++++ 5 files changed, 63 insertions(+) create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json create mode 100644 crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json new file mode 100644 index 000000000..ec9bd7d10 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json @@ -0,0 +1 @@ +{ "name": "@test/sibling-pkg", "version": "0.0.0" } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt new file mode 100644 index 000000000..e620d96dd --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt @@ -0,0 +1 @@ +sibling diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json new file mode 100644 index 000000000..416a32370 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir rebuild src/data.txt dist && vtt cp dist/out.txt dist/copied.txt && vtt cp src/data.txt dist/late.txt" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml index 001ba1485..c29f31f37 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml @@ -37,3 +37,15 @@ steps = [ ["vt", "run", "task"], ["vtt", "print-file", ".cache/state.txt"], ] + +[[e2e]] +name = "reading_inside_own_output_tree_still_caches" +comment = """ +A compound command's `&&` segments are separate cached tasks. Here the middle segment reads a file inside its own package's output directory while the last segment writes another file there, which is the shape of emdash's admin build: `tsdown` reads `dist/locales//messages.mjs` and a later `locale:copy` step rewrites those files. Fingerprinting content a sibling task rewrites can never settle, so a read is treated as the task's own derived state when version control calls it derived *and* it sits under a directory the same task wrote into. Requiring both is what keeps `node_modules//dist` a real input, so changing a dependency still invalidates its consumers. +""" +cwd = "packages/sibling-pkg" +steps = [ + ["vt", "run", "task"], + ["vt", "run", "task"], + ["vt", "run", "task"], +] diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md new file mode 100644 index 000000000..97dfc5569 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md @@ -0,0 +1,42 @@ +# reading_inside_own_output_tree_still_caches + +A compound command's `&&` segments are separate cached tasks. Here the middle segment reads a file inside its own package's output directory while the last segment writes another file there, which is the shape of emdash's admin build: `tsdown` reads `dist/locales//messages.mjs` and a later `locale:copy` step rewrites those files. Fingerprinting content a sibling task rewrites can never settle, so a read is treated as the task's own derived state when version control calls it derived *and* it sits under a directory the same task wrote into. Requiring both is what keeps `node_modules//dist` a real input, so changing a dependency still invalidates its consumers. + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt + +--- +vt run: 0/3 cache hit (0%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` From e53d6d817f69d7970668b1b20371c27a24885690 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:39:13 +0800 Subject: [PATCH 11/21] feat(fspy): track renames, deletes and create flags on Windows Rename and delete reach the kernel through NtSetInformationFile on Windows rather than through dedicated syscalls, so a single detour on that entry point covers what rename, unlink and rmdir cover on Unix. Without it a tool that publishes atomically is invisible: the write lands on a temporary path that no longer exists at archive time, and the destination is never seen to change. NtCreateFile now also folds CreateDisposition into the access mode. Disposition is Windows' O_CREAT/O_TRUNC/O_EXCL, and without it a handle opened for writing cannot be told apart from one that actually replaces content -- only the latter is a mutation. Both interceptions record the call's arguments and never its result, matching the Unix backends. The Linux seccomp supervisor is notified before the syscall runs and can never observe outcomes, so an outcome-dependent rule would hold on some platforms and not others. Co-authored-by: Claude Opus 5 --- .../src/windows/convert.rs | 11 ++ .../src/windows/detours/nt.rs | 105 ++++++++++++++++- .../src/windows/winapi_utils.rs | 107 +++++++++++++++++- 3 files changed, 220 insertions(+), 3 deletions(-) diff --git a/crates/fspy_preload_windows/src/windows/convert.rs b/crates/fspy_preload_windows/src/windows/convert.rs index 72a095786..4f6946fa6 100644 --- a/crates/fspy_preload_windows/src/windows/convert.rs +++ b/crates/fspy_preload_windows/src/windows/convert.rs @@ -34,6 +34,17 @@ pub trait ToAbsolutePath { ) -> winsafe::SysResult; } +/// An already-resolved absolute path, used for a rename destination that was +/// reconstructed from `FILE_RENAME_INFORMATION` rather than taken from a handle. +impl ToAbsolutePath for &U16Str { + unsafe fn to_absolute_path) -> winsafe::SysResult>( + self, + f: F, + ) -> winsafe::SysResult { + f(Some(self)) + } +} + impl ToAbsolutePath for HANDLE { unsafe fn to_absolute_path) -> winsafe::SysResult>( self, diff --git a/crates/fspy_preload_windows/src/windows/detours/nt.rs b/crates/fspy_preload_windows/src/windows/detours/nt.rs index 12fb8c050..2b49c3d28 100644 --- a/crates/fspy_preload_windows/src/windows/detours/nt.rs +++ b/crates/fspy_preload_windows/src/windows/detours/nt.rs @@ -27,6 +27,7 @@ use crate::windows::{ client::global_client, convert::{ToAbsolutePath, ToAccessMode}, detour::{Detour, DetourAny}, + winapi_utils::{create_disposition_to_mode, handle_is_directory, rename_target_path}, }; // CreateProcess ultimately asks NtCreateUserProcess to open the executable image. Some Windows @@ -191,8 +192,17 @@ static DETOUR_NT_CREATE_FILE: Detour< ea_buffer: PVOID, ea_length: ULONG, ) -> HFILE { + // CreateDisposition is Windows' O_CREAT/O_TRUNC/O_EXCL. Without + // it a handle opened for writing is indistinguishable from one + // that actually replaces content, and only the latter is a + // mutation. + let mode = { + // SAFETY: converting the access mask through the FFI-aware trait + let access = unsafe { desired_access.to_access_mode() }; + access | create_disposition_to_mode(create_disposition) + }; // SAFETY: intercepting file open to record access before forwarding to real function - unsafe { handle_open(desired_access, object_attributes) }; + unsafe { handle_open(mode, object_attributes) }; // SAFETY: calling the original NtCreateFile with all original arguments unsafe { @@ -528,4 +538,97 @@ pub const DETOURS: &[DetourAny] = &[ DETOUR_NT_QUERY_INFORMATION_BY_NAME.as_any(), DETOUR_NT_QUERY_DIRECTORY_FILE.as_any(), DETOUR_NT_QUERY_DIRECTORY_FILE_EX.as_any(), + DETOUR_NT_SET_INFORMATION_FILE.as_any(), ]; + +/// Rename and delete both travel through `NtSetInformationFile` on Windows +/// rather than through dedicated syscalls, so this one detour covers what +/// `rename`, `unlink` and `rmdir` cover on Unix. +/// +/// Without it, a tool that publishes atomically is invisible: the write lands on +/// a temporary path that no longer exists, and the destination is never seen to +/// change. Directory renames are worse, because a build that stages into +/// `dist.tmp` and swaps it into place reports no write for any real output. +/// +/// Like every other interception here, this records the call's arguments and +/// never its result. The Linux seccomp backend cannot observe outcomes at all, so +/// an outcome-dependent rule would hold on some platforms and not others. +pub static DETOUR_NT_SET_INFORMATION_FILE: Detour< + unsafe extern "system" fn( + file_handle: HANDLE, + io_status_block: PIO_STATUS_BLOCK, + file_information: PVOID, + length: ULONG, + file_information_class: FILE_INFORMATION_CLASS, + ) -> NTSTATUS, +> = + // SAFETY: initializing Detour with the real NtSetInformationFile and our replacement + unsafe { + Detour::new(c"NtSetInformationFile", ntapi::ntioapi::NtSetInformationFile, { + unsafe extern "system" fn new_nt_set_information_file( + file_handle: HANDLE, + io_status_block: PIO_STATUS_BLOCK, + file_information: PVOID, + length: ULONG, + file_information_class: FILE_INFORMATION_CLASS, + ) -> NTSTATUS { + // Values from the NtSetInformationFile contract. + const FILE_RENAME_INFORMATION_CLASS: FILE_INFORMATION_CLASS = 10; + const FILE_RENAME_INFORMATION_EX_CLASS: FILE_INFORMATION_CLASS = 65; + const FILE_DISPOSITION_INFORMATION_CLASS: FILE_INFORMATION_CLASS = 13; + const FILE_DISPOSITION_INFORMATION_EX_CLASS: FILE_INFORMATION_CLASS = 64; + + match file_information_class { + FILE_RENAME_INFORMATION_CLASS | FILE_RENAME_INFORMATION_EX_CLASS => { + // SAFETY: file_handle is supplied by the caller + let is_directory = unsafe { handle_is_directory(file_handle) }; + let dir_flag = + if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: file_handle refers to the rename source, still + // valid before the call + unsafe { + handle_open( + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag, + file_handle, + ); + } + // SAFETY: file_information holds `length` bytes of + // FILE_RENAME_INFORMATION per the caller's contract, and + // the helper bounds-checks against `length` + let target = + unsafe { rename_target_path(file_information.cast::(), length) }; + if let Some(target) = target { + // SAFETY: target is an owned null-terminated wide string + unsafe { + handle_open( + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + target.as_ucstr().as_ustr(), + ); + } + } + } + FILE_DISPOSITION_INFORMATION_CLASS | FILE_DISPOSITION_INFORMATION_EX_CLASS => { + // SAFETY: file_handle is supplied by the caller + let is_directory = unsafe { handle_is_directory(file_handle) }; + let dir_flag = + if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: file_handle is still valid before the call + unsafe { handle_open(AccessMode::DELETED | dir_flag, file_handle) }; + } + _ => {} + } + + // SAFETY: calling the original NtSetInformationFile with all original arguments + unsafe { + (DETOUR_NT_SET_INFORMATION_FILE.real())( + file_handle, + io_status_block, + file_information, + length, + file_information_class, + ) + } + } + new_nt_set_information_file + }) + }; diff --git a/crates/fspy_preload_windows/src/windows/winapi_utils.rs b/crates/fspy_preload_windows/src/windows/winapi_utils.rs index 38eba1c19..9451fa29d 100644 --- a/crates/fspy_preload_windows/src/windows/winapi_utils.rs +++ b/crates/fspy_preload_windows/src/windows/winapi_utils.rs @@ -1,8 +1,11 @@ -use std::slice; +use std::{ + mem::{offset_of, size_of}, + slice, +}; use fspy_shared::ipc::AccessMode; use smallvec::SmallVec; -use widestring::{U16CStr, U16Str}; +use widestring::{U16CStr, U16CString, U16Str}; use winapi::{ ctypes::c_long, shared::{ @@ -103,6 +106,35 @@ pub const fn access_mask_to_mode(desired_access: ACCESS_MASK) -> AccessMode { } } +/// Translate `NtCreateFile`'s `CreateDisposition` into creation and truncation +/// intent. +/// +/// This is Windows' equivalent of `O_CREAT`, `O_TRUNC` and `O_EXCL`, and the +/// consumer needs it for the same reason: a handle opened for writing is only +/// capability, while replacing or newly creating a file is a mutation. +pub const fn create_disposition_to_mode(create_disposition: u32) -> AccessMode { + // Values from the NtCreateFile contract. Named locally because ntapi + // exposes them as plain constants of varying integer types. + const FILE_SUPERSEDE: u32 = 0; + const FILE_CREATE: u32 = 2; + const FILE_OPEN_IF: u32 = 3; + const FILE_OVERWRITE: u32 = 4; + const FILE_OVERWRITE_IF: u32 = 5; + + match create_disposition { + // Replaces the file if it exists, creates it otherwise. + FILE_SUPERSEDE | FILE_OVERWRITE_IF => AccessMode::CREATE.union(AccessMode::TRUNCATE), + // Fails if the file already exists, so the handle refers to new content. + FILE_CREATE => AccessMode::CREATE.union(AccessMode::EXCLUSIVE), + // Creates only when absent, leaving existing content in place. + FILE_OPEN_IF => AccessMode::CREATE, + // Requires the file to exist, then discards its contents. + FILE_OVERWRITE => AccessMode::TRUNCATE, + // FILE_OPEN and anything unrecognized: no creation, no truncation. + _ => AccessMode::empty(), + } +} + pub struct HeapPath(PWSTR); impl HeapPath { #[must_use] @@ -136,6 +168,77 @@ pub fn combine_paths(path1: &U16CStr, path2: &U16CStr) -> winsafe::SysResult bool { + use winapi::um::{ + fileapi::{BY_HANDLE_FILE_INFORMATION, GetFileInformationByHandle}, + winnt::FILE_ATTRIBUTE_DIRECTORY, + }; + + // SAFETY: an all-zero BY_HANDLE_FILE_INFORMATION is a valid initial value + let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { core::mem::zeroed() }; + // SAFETY: handle comes from the interposed caller and info is a valid owned struct + if unsafe { GetFileInformationByHandle(handle, &raw mut info) } == 0 { + return false; + } + info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0 +} + +/// The destination path carried by a `FILE_RENAME_INFORMATION` buffer. +/// +/// The struct is variable-length: a fixed header followed by the target name. +/// When `RootDirectory` is set the name is relative to that directory, matching +/// `renameat` on Unix. +/// +/// # Safety +/// +/// `info` must point to at least `length` readable bytes holding a +/// `FILE_RENAME_INFORMATION`, as supplied by the `NtSetInformationFile` caller. +#[expect( + clippy::cast_ptr_alignment, + reason = "the kernel requires callers to pass a correctly aligned FILE_RENAME_INFORMATION" +)] +pub unsafe fn rename_target_path(info: *const u8, length: u32) -> Option { + use ntapi::ntioapi::FILE_RENAME_INFORMATION; + + if info.is_null() || (length as usize) < size_of::() { + return None; + } + // SAFETY: the caller guarantees `info` holds a FILE_RENAME_INFORMATION + let rename = unsafe { &*info.cast::() }; + let name_bytes = rename.FileNameLength as usize; + if name_bytes == 0 || !name_bytes.is_multiple_of(2) { + return None; + } + let name_offset = offset_of!(FILE_RENAME_INFORMATION, FileName); + if name_offset.saturating_add(name_bytes) > length as usize { + return None; + } + // SAFETY: bounds checked against `length` above; FileName is a wide string + let name = + unsafe { std::slice::from_raw_parts(info.add(name_offset).cast::(), name_bytes / 2) }; + let target = U16Str::from_slice(name); + + let is_absolute = + name.first() == Some(&u16::from(b'\\')) || name.get(1) == Some(&u16::from(b':')); + if is_absolute || rename.RootDirectory.is_null() { + return Some(U16CString::from_ustr_truncate(target)); + } + // Relative to RootDirectory, so resolve it the same way an open would. + // SAFETY: RootDirectory is a directory handle supplied by the caller + let mut root = unsafe { get_path_name(rename.RootDirectory) }.ok()?; + root.push(0); + // SAFETY: a null terminator was just pushed + let root = unsafe { U16CStr::from_ptr_str(root.as_ptr()) }; + let combined = combine_paths(root, U16CString::from_ustr_truncate(target).as_ucstr()).ok()?; + Some(U16CString::from_ustr_truncate(combined.to_u16_str())) +} + #[cfg(test)] mod tests { use std::{ From fba6559bab12d1c731b5a7f2ea674aa677d03158 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:39:23 +0800 Subject: [PATCH 12/21] fix(lint): satisfy Windows-only clippy lints on the current toolchain Both items are gated behind cfg(windows), so the native macOS and Linux clippy runs never reach them and these failures only surface under `just lint-windows`. Neither is related to file-tracking work; the cross-target lint gate is red on main without them. `unused_async_trait_impl` is a newer sibling of `unused_async`, which FspySpawner::spawn already expects for the same reason: the async signature is required by the SpyImpl trait. Co-authored-by: Claude Opus 5 --- crates/fspy/src/windows/mod.rs | 6 +++++- crates/vite_powershell/src/lib.rs | 7 ++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/fspy/src/windows/mod.rs b/crates/fspy/src/windows/mod.rs index 8081e1298..fd235ece2 100644 --- a/crates/fspy/src/windows/mod.rs +++ b/crates/fspy/src/windows/mod.rs @@ -66,7 +66,11 @@ impl SpyImpl { Ok(Self { ansi_dll_path_with_nul: ansi_dll_path_with_nul.into() }) } - #[expect(clippy::unused_async, reason = "async signature required by SpyImpl trait")] + #[expect( + clippy::unused_async, + clippy::unused_async_trait_impl, + reason = "async signature required by SpyImpl trait" + )] pub(crate) async fn spawn( &self, mut command: Command, diff --git a/crates/vite_powershell/src/lib.rs b/crates/vite_powershell/src/lib.rs index 2819cdf35..52e1b7a2d 100644 --- a/crates/vite_powershell/src/lib.rs +++ b/crates/vite_powershell/src/lib.rs @@ -27,9 +27,10 @@ use vite_path::{AbsolutePath, AbsolutePathBuf}; pub const POWERSHELL_PREFIX: &[&str] = &["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File"]; -/// Cached location of the `PowerShell` host. Prefers cross-platform -/// `pwsh.exe` when present, falling back to the Windows built-in -/// `powershell.exe`. Returns `None` on non-Windows or when neither host +/// Cached location of the `PowerShell` host. +/// +/// Prefers cross-platform `pwsh.exe` when present, falling back to the Windows +/// built-in `powershell.exe`. Returns `None` on non-Windows or when neither host /// is on `PATH`. /// /// Cached as `Arc` so callers that want shared ownership From c7fea038290a23353c4e2d3e8ad66dd0b1f30b6c Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:39:30 +0800 Subject: [PATCH 13/21] style: apply repo formatters to the auto-tracking changes Pure formatting from `just fmt` (rustfmt wrapping, module ordering, prettier on the new fixtures and DECISIONS.md). No behaviour change. Co-authored-by: Claude Opus 5 --- DECISIONS.md | 10 +-- crates/fspy_preload_unix/src/client/mod.rs | 2 - .../src/interceptions/mutate.rs | 13 +--- .../src/session/execute/cache_update.rs | 12 +-- .../vite_task/src/session/execute/classify.rs | 10 ++- crates/vite_task/src/session/execute/mod.rs | 2 +- crates/vite_task_bin/src/vtt/main.rs | 4 +- .../auto_output_tracking/package.json | 6 +- .../packages/atomic-pkg/package.json | 5 +- .../packages/clean-pkg/package.json | 5 +- .../packages/sibling-pkg/package.json | 5 +- .../packages/state-pkg/package.json | 5 +- .../auto_output_tracking/snapshots.toml | 74 ++++++++++++++----- 13 files changed, 97 insertions(+), 56 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 760b11432..38d593de4 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -54,7 +54,7 @@ a mutation. - Atomic writers create temporaries with `O_CREAT | O_EXCL` and publish by rename, so both halves are still detected. - Biome's warm run opens a clean source `O_RDWR` with no truncation and does not - write. It is now correctly *not* a mutation, which is the false positive the + write. It is now correctly _not_ a mutation, which is the false positive the rule exists to remove. - Lock files across cargo, rustc and Parcel are opened `O_RDWR | O_CREAT` without truncation and only flocked, so they stop being collected. @@ -79,7 +79,7 @@ directory listings including all 256 of Go's cache shards. **Drift.** D1 committed to all signals on all three backends. The seccomp supervisor responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` (`fspy_seccomp_unotify/src/supervisor/listener.rs:50`), so it is notified -*before* the syscall runs and never learns the result. Success-dependent signals +_before_ the syscall runs and never learns the result. Success-dependent signals are therefore not obtainable there without emulating each syscall in the supervisor, which would mean reproducing the target's cwd, dirfd and credentials. @@ -100,7 +100,7 @@ directory as an input. Both cost cache hits, not correctness. ## D8 — Guard: an unexplained missing mutation blocks caching -**Drift.** One missing signal is *not* safe on its own. If a rename goes +**Drift.** One missing signal is _not_ safe on its own. If a rename goes unobserved, the write lands on a staging path that no longer exists at archive time, rule 6 drops it, and the archive comes out empty — which restores nothing on a cache hit and leaves a wrong tree. That is exactly the `atomic-dist-swap` @@ -120,7 +120,7 @@ missed cache entry, never a wrong tree. **Instruction.** Do not build rules on whether a call succeeded. **What this invalidates.** D1 through D8 assumed the tracer could report -outcomes, which drove reporting *after* the real call so `errno` was known. That +outcomes, which drove reporting _after_ the real call so `errno` was known. That is now reverted: - `AccessMode::FAILED` is removed, along with the after-the-call reporting in @@ -243,7 +243,7 @@ separate cached task: node --run locale:compile && tsdown && node --run locale:copy && npx tailwindcss ``` -`tsdown` *reads* `dist/locales//messages.mjs`, and the later +`tsdown` _reads_ `dist/locales//messages.mjs`, and the later `locale:copy` segment rewrites those files. So tsdown fingerprints content that a sibling task changes afterwards, and the fingerprint can never settle. diff --git a/crates/fspy_preload_unix/src/client/mod.rs b/crates/fspy_preload_unix/src/client/mod.rs index d7511eeb2..daae12f5a 100644 --- a/crates/fspy_preload_unix/src/client/mod.rs +++ b/crates/fspy_preload_unix/src/client/mod.rs @@ -158,8 +158,6 @@ pub unsafe fn handle_open(path: impl ToAbsolutePath, mode: impl ToAccessMode) { }); } - - #[cfg(not(test))] #[ctor::ctor(unsafe)] fn init_client() { diff --git a/crates/fspy_preload_unix/src/interceptions/mutate.rs b/crates/fspy_preload_unix/src/interceptions/mutate.rs index 615ca0a53..f9f5d87ea 100644 --- a/crates/fspy_preload_unix/src/interceptions/mutate.rs +++ b/crates/fspy_preload_unix/src/interceptions/mutate.rs @@ -35,7 +35,8 @@ unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { // SAFETY: an all-zero stat is a valid initial value; fstatat overwrites it let mut stat_buf: libc::stat = unsafe { core::mem::zeroed() }; // SAFETY: path is a valid C string pointer and stat_buf is a valid, owned stat struct - let result = unsafe { libc::fstatat(dirfd, path, &raw mut stat_buf, libc::AT_SYMLINK_NOFOLLOW) }; + let result = + unsafe { libc::fstatat(dirfd, path, &raw mut stat_buf, libc::AT_SYMLINK_NOFOLLOW) }; result == 0 && (stat_buf.st_mode & libc::S_IFMT) == libc::S_IFDIR } @@ -110,10 +111,7 @@ unsafe extern "C" fn renameatx_np( if flags & libc::RENAME_SWAP != 0 { // SAFETY: old_path is a valid C string pointer provided by the caller unsafe { - handle_open( - PathAt(old_dirfd, old_path), - AccessMode::RENAME_TO | AccessMode::WRITE, - ); + handle_open(PathAt(old_dirfd, old_path), AccessMode::RENAME_TO | AccessMode::WRITE); } } // SAFETY: forwarding the caller's arguments to the original renameatx_np() @@ -141,10 +139,7 @@ unsafe extern "C" fn renameat2( if flags & libc::RENAME_EXCHANGE != 0 { // SAFETY: old_path is a valid C string pointer provided by the caller unsafe { - handle_open( - PathAt(old_dirfd, old_path), - AccessMode::RENAME_TO | AccessMode::WRITE, - ); + handle_open(PathAt(old_dirfd, old_path), AccessMode::RENAME_TO | AccessMode::WRITE); } } // SAFETY: forwarding the caller's arguments to the original renameat2() diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index 046511993..b07ff8b9b 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -15,11 +15,9 @@ use super::{ glob, spawn::ChildOutcome, }; -use crate::{ - session::{ - cache::{CacheEntryValue, ExecutionCache, archive}, - event::{CacheErrorKind, CacheNotUpdatedReason, CacheUpdateStatus, ExecutionError}, - }, +use crate::session::{ + cache::{CacheEntryValue, ExecutionCache, archive}, + event::{CacheErrorKind, CacheNotUpdatedReason, CacheUpdateStatus, ExecutionError}, }; /// Post-execution summary of what fspy observed for a single task. Fields are @@ -264,9 +262,7 @@ fn observe_fspy( .modified_input .filter(|path| !excluded_from_inputs(path)) .or_else(|| { - classification - .unexplained_mutation - .filter(|path| !excluded_from_outputs(path)) + classification.unexplained_mutation.filter(|path| !excluded_from_outputs(path)) }); if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index f72597047..b57a5ff8c 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -216,7 +216,8 @@ fn relocate_directory_renames( else { continue; }; - let Ok(moved) = RelativePathBuf::new(vite_str::format!("{destination}/{tail}").as_str()) + let Ok(moved) = + RelativePathBuf::new(vite_str::format!("{destination}/{tail}").as_str()) else { continue; }; @@ -238,8 +239,7 @@ pub fn classify( let mut candidates: Vec<(&RelativePathBuf, &PathHistory)> = histories.iter().map(|(path, history)| (*path, history)).collect(); - let relocated_entries: Vec<(&RelativePathBuf, &PathHistory)> = - relocated.iter().collect(); + let relocated_entries: Vec<(&RelativePathBuf, &PathHistory)> = relocated.iter().collect(); candidates.extend(relocated_entries); // Directories that were renamed away or removed. Anything that used to live @@ -323,7 +323,9 @@ pub fn classify( record_input(&mut classification, path, history, already_fingerprinted); } } - (false, true) => record_input(&mut classification, path, history, already_fingerprinted), + (false, true) => { + record_input(&mut classification, path, history, already_fingerprinted) + } (false, false) => {} } } diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs index 4bf97fe95..f0eeb69ca 100644 --- a/crates/vite_task/src/session/execute/mod.rs +++ b/crates/vite_task/src/session/execute/mod.rs @@ -1,9 +1,9 @@ mod cache_update; #[cfg(fspy)] mod classify; +pub mod fingerprint; #[cfg(fspy)] mod gitignore; -pub mod fingerprint; pub mod glob; mod hash; pub mod pipe; diff --git a/crates/vite_task_bin/src/vtt/main.rs b/crates/vite_task_bin/src/vtt/main.rs index e295c510a..939d921c9 100644 --- a/crates/vite_task_bin/src/vtt/main.rs +++ b/crates/vite_task_bin/src/vtt/main.rs @@ -20,10 +20,10 @@ mod print_color; mod print_cwd; mod print_env; mod print_file; -mod read_stdin; -mod replace_file_content; mod publish_dir; +mod read_stdin; mod rename; +mod replace_file_content; mod rm; #[cfg(target_os = "linux")] mod small_dev_shm; diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json index 417e4e013..bd710e911 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json @@ -1 +1,5 @@ -{ "name": "auto-output-tracking", "private": true, "version": "0.0.0" } +{ + "name": "auto-output-tracking", + "version": "0.0.0", + "private": true +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json index c8f1d2186..7d075639b 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json @@ -1 +1,4 @@ -{ "name": "@test/atomic-pkg", "version": "0.0.0" } +{ + "name": "@test/atomic-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json index b55cbfa65..5fdf310b4 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json @@ -1 +1,4 @@ -{ "name": "@test/clean-pkg", "version": "0.0.0" } +{ + "name": "@test/clean-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json index ec9bd7d10..219163dc7 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json @@ -1 +1,4 @@ -{ "name": "@test/sibling-pkg", "version": "0.0.0" } +{ + "name": "@test/sibling-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json index 7f1a13933..ec1a34c57 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json @@ -1 +1,4 @@ -{ "name": "@test/state-pkg", "version": "0.0.0" } +{ + "name": "@test/state-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml index c29f31f37..8d02cfe17 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml @@ -5,11 +5,32 @@ A task that stages output under `dist.tmp` and renames the directory into place """ cwd = "packages/atomic-pkg" steps = [ - ["vt", "run", "task"], - ["vt", "run", "task"], - ["vtt", "rm", "-rf", "dist"], - ["vt", "run", "task"], - ["vtt", "print-file", "dist/out.txt"], + [ + "vt", + "run", + "task", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "rm", + "-rf", + "dist", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "print-file", + "dist/out.txt", + ], ] [[e2e]] @@ -19,11 +40,32 @@ Build tools empty their output directory before writing to it, which means they """ cwd = "packages/clean-pkg" steps = [ - ["vt", "run", "task"], - ["vt", "run", "task"], - ["vtt", "rm", "-rf", "dist"], - ["vt", "run", "task"], - ["vtt", "print-file", "dist/out.txt"], + [ + "vt", + "run", + "task", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "rm", + "-rf", + "dist", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "print-file", + "dist/out.txt", + ], ] [[e2e]] @@ -32,11 +74,7 @@ comment = """ A path a task reads and then rewrites is either a source it fixed up or state it manages, and access mechanics cannot tell those apart. Version control can: this file is gitignored, so it is derived state and belongs in the archive. Compare `input_read_write_not_cached`, where the same access pattern on a tracked source blocks caching instead. """ cwd = "packages/state-pkg" -steps = [ - ["vt", "run", "task"], - ["vt", "run", "task"], - ["vtt", "print-file", ".cache/state.txt"], -] +steps = [["vt", "run", "task"], ["vt", "run", "task"], ["vtt", "print-file", ".cache/state.txt"]] [[e2e]] name = "reading_inside_own_output_tree_still_caches" @@ -44,8 +82,4 @@ comment = """ A compound command's `&&` segments are separate cached tasks. Here the middle segment reads a file inside its own package's output directory while the last segment writes another file there, which is the shape of emdash's admin build: `tsdown` reads `dist/locales//messages.mjs` and a later `locale:copy` step rewrites those files. Fingerprinting content a sibling task rewrites can never settle, so a read is treated as the task's own derived state when version control calls it derived *and* it sits under a directory the same task wrote into. Requiring both is what keeps `node_modules//dist` a real input, so changing a dependency still invalidates its consumers. """ cwd = "packages/sibling-pkg" -steps = [ - ["vt", "run", "task"], - ["vt", "run", "task"], - ["vt", "run", "task"], -] +steps = [["vt", "run", "task"], ["vt", "run", "task"], ["vt", "run", "task"]] From c86a04e8bfb49f012b6f6b5c0765a0a7b4a0fa6b Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:39:48 +0800 Subject: [PATCH 14/21] fix(typo): unparseable -> unparsable Co-authored-by: Claude Opus 5 --- crates/vite_task/src/session/execute/gitignore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vite_task/src/session/execute/gitignore.rs b/crates/vite_task/src/session/execute/gitignore.rs index 64d8ceddf..a18f92abe 100644 --- a/crates/vite_task/src/session/execute/gitignore.rs +++ b/crates/vite_task/src/session/execute/gitignore.rs @@ -35,7 +35,7 @@ impl WorkspaceGitignore { /// it walks the ones it is given, and a missing file is simply no rules. /// /// A malformed pattern is not fatal. Failing the whole task because one line - /// of a `.gitignore` is unparseable would be worse than treating that line as + /// of a `.gitignore` is unparsable would be worse than treating that line as /// absent, and the fallback direction is safe: fewer ignore rules means more /// paths look tracked, which means fewer runs are cached. pub fn open(workspace_root: &AbsolutePath) -> Self { From cefe2ebd9ae080cb7ddf78ccfec4c05131b32cb6 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:40:40 +0800 Subject: [PATCH 15/21] fix(lint): add the semicolon dropped while reformatting classify Co-authored-by: Claude Opus 5 --- crates/vite_task/src/session/execute/classify.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs index b57a5ff8c..0ec0b5b62 100644 --- a/crates/vite_task/src/session/execute/classify.rs +++ b/crates/vite_task/src/session/execute/classify.rs @@ -324,7 +324,7 @@ pub fn classify( } } (false, true) => { - record_input(&mut classification, path, history, already_fingerprinted) + record_input(&mut classification, path, history, already_fingerprinted); } (false, false) => {} } From 71f8fa6d772504e6dfbffa2c5b312c152a8ea9ae Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:45:25 +0800 Subject: [PATCH 16/21] docs(changelog): record the auto-tracking classification fix Co-authored-by: Claude Opus 5 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c5c30f41..ddf03ef3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** Automatic tracking now classifies files that a task both reads and writes, so tasks that publish an output directory by renaming a staging directory into place, or that read their own generated files, cache correctly without hand-written `input`/`output` patterns ([#571](https://github.com/voidzero-dev/vite-task/pull/571)). - **Added** Tasks now run with `VP_RUN=1` set, so tools can tell they are running under `vp run` instead of being invoked directly ([#570](https://github.com/voidzero-dev/vite-task/pull/570)). - **Fixed** The task cache now supports much larger automatically tracked input sets without hitting wincode's default 4 MiB sequence preallocation limit ([#554](https://github.com/voidzero-dev/vite-task/pull/554)). - **Fixed** npm workspace patterns beginning with `./` now discover matching packages correctly ([vite-plus#2201](https://github.com/voidzero-dev/vite-plus/issues/2201), [#547](https://github.com/voidzero-dev/vite-task/pull/547)). From b81f374708146cc95f323bb51d89a603e4b03b16 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:56:01 +0800 Subject: [PATCH 17/21] fix(cache): treat node_modules as derived without a .gitignore Vitest reads and then rewrites `node_modules/.vite/vitest/*/results.json` on every run. Resolving that read-first overlap through gitignore alone called it a modified input, so the task could never cache: the browser fixture reported "not cached because it modified its input" on all three runs. The gitignore proxy assumed a workspace has a `.gitignore` covering its derived state. The fixture has none, and a package or a non-repository workspace need not either. `node_modules` is installed rather than authored and `.git` is the repository itself, so neither is ever an authored source and neither needs a rule to say so. Co-authored-by: Claude Opus 5 --- .../src/session/execute/gitignore.rs | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/crates/vite_task/src/session/execute/gitignore.rs b/crates/vite_task/src/session/execute/gitignore.rs index a18f92abe..86ef9e18b 100644 --- a/crates/vite_task/src/session/execute/gitignore.rs +++ b/crates/vite_task/src/session/execute/gitignore.rs @@ -12,16 +12,27 @@ //! output globs override the answer, so the proxy only has to be right for paths //! nobody declared. //! -//! When there is no repository, every read-first overlap resolves to *input*. -//! That is the safe direction: the task modified something it declared as a -//! dependency, so the run is not cached. Availability degrades, correctness does -//! not. +//! When there is no repository, every read-first overlap resolves to *input*, +//! apart from the always-derived directories below. That is the safe direction: +//! the task modified something it declared as a dependency, so the run is not +//! cached. Availability degrades, correctness does not. #![cfg(fspy)] use ignore::gitignore::{Gitignore, GitignoreBuilder}; use vite_path::{AbsolutePath, RelativePathBuf}; +/// Directories that hold derived state in every JavaScript workspace, whether or +/// not a `.gitignore` says so. +/// +/// `node_modules` is installed rather than authored, and `.git` is the repository +/// itself. Relying on a `.gitignore` to say so is not enough: a package with no +/// `.gitignore` of its own — or a workspace that is not a repository at all — +/// would classify a tool's own cache under `node_modules` as a modified input and +/// then never cache. Vitest keeps `node_modules/.vite/vitest/*/results.json`, +/// which it reads and then rewrites on every run. +const ALWAYS_IGNORED: &[&str] = &["node_modules", ".git"]; + /// Gitignore matching rooted at the workspace. pub struct WorkspaceGitignore { matcher: Option, @@ -52,6 +63,14 @@ impl WorkspaceGitignore { /// `false` when there is no matcher at all, which is the input-leaning /// answer. pub fn is_ignored(&self, path: &RelativePathBuf) -> bool { + if path + .as_path() + .iter() + .any(|component| ALWAYS_IGNORED.contains(&component.to_string_lossy().as_ref())) + { + return true; + } + let Some(matcher) = &self.matcher else { return false; }; @@ -92,4 +111,32 @@ mod tests { "tracked sources must not be reported as ignored" ); } + + /// A workspace with no `.gitignore` must still recognise `node_modules` as + /// derived. Vitest reads and then rewrites its own + /// `node_modules/.vite/vitest/*/results.json`, and calling that a modified + /// input stops the task from ever caching. + #[test] + fn node_modules_is_derived_without_any_gitignore() { + let temp = TempDir::new().unwrap(); + let root = AbsolutePathBuf::new(temp.path().to_path_buf()).unwrap(); + + let gitignore = WorkspaceGitignore::open(&root); + + assert!( + gitignore.is_ignored( + &RelativePathBuf::new("node_modules/.vite/vitest/abc/results.json").unwrap() + ), + "a tool's own cache under node_modules must not become a modified input" + ); + assert!( + gitignore + .is_ignored(&RelativePathBuf::new("packages/auth/node_modules/x/cache").unwrap()), + "nested node_modules must be covered too" + ); + assert!( + !gitignore.is_ignored(&RelativePathBuf::new("src/index.ts").unwrap()), + "sources must still be tracked when there is no .gitignore" + ); + } } From 44547626c351c7f5c5d1242a46ea35ff2c56f7a7 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 22:56:11 +0800 Subject: [PATCH 18/21] test(fspy): compare access kinds, not the runtime's open flags `assert_contains` compared the accumulated mode for exact equality, so adding CREATE and TRUNCATE broke every write assertion in node_fs. Spelling the new flags out per test would assert the runtime's open flags rather than the behaviour under test, and they genuinely differ: `writeFileSync` truncates on Node and Deno but not on Bun. These modifiers now only participate when a test names one explicitly, leaving the read versus write distinction as strict as before. Co-authored-by: Claude Opus 5 --- crates/fspy/tests/test_utils/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/fspy/tests/test_utils/mod.rs b/crates/fspy/tests/test_utils/mod.rs index cfa46c4a9..78ea6d49c 100644 --- a/crates/fspy/tests/test_utils/mod.rs +++ b/crates/fspy/tests/test_utils/mod.rs @@ -17,6 +17,16 @@ use fspy::{AccessMode, PathAccessIterable}; )] pub use subprocess_test::command_for_fn; +/// Flags describing how an access was requested rather than what kind of access +/// it was. They differ between runtimes for the same API: `writeFileSync` +/// truncates on Node and Deno but not on Bun. Spelling them out in every test +/// would assert the runtime's open flags rather than the behaviour under test, so +/// they only participate when a test names one explicitly. +const MODIFIERS: AccessMode = AccessMode::CREATE + .union(AccessMode::TRUNCATE) + .union(AccessMode::EXCLUSIVE) + .union(AccessMode::IS_DIR); + /// # Panics /// /// Panics if the expected path access is not found or has the wrong mode. @@ -46,6 +56,10 @@ pub fn assert_contains( actual_mode.remove(AccessMode::READ); } + if !expected_mode.intersects(MODIFIERS) { + actual_mode.remove(MODIFIERS); + } + assert_eq!( expected_mode, actual_mode, From 4c42331ee956cddc54dedae509790b5649ab43c7 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Sun, 26 Jul 2026 23:06:40 +0800 Subject: [PATCH 19/21] feat(fspy): intercept rename and delete in the seccomp backend musl builds no preload library and rely entirely on seccomp, which only watched open, stat, getdents and execve. A task that publishes its output by renaming a staging directory into place therefore looked like it wrote nothing that still existed, and the atomic-publish case reported "not cached because it modified its input" on musl while passing everywhere else. Source and destination both come from the call's arguments, so this adds no outcome dependency: the supervisor still responds with SECCOMP_USER_NOTIF_FLAG_CONTINUE and never learns whether the call succeeded. Whether the source is a directory is read from the filesystem before the syscall runs, which is existing state rather than a result, and it decides whether writes under the old subtree are re-attributed. renameat2 needs a fifth argument, so FromNotify gains a 5-tuple. Co-authored-by: Claude Opus 5 --- crates/fspy/src/unix/syscall_handler/mod.rs | 104 ++++++++++++++++++ .../fspy/src/unix/syscall_handler/mutate.rs | 64 +++++++++++ .../src/supervisor/handler/arg.rs | 19 ++++ 3 files changed, 187 insertions(+) create mode 100644 crates/fspy/src/unix/syscall_handler/mutate.rs diff --git a/crates/fspy/src/unix/syscall_handler/mod.rs b/crates/fspy/src/unix/syscall_handler/mod.rs index 722410779..eb5de63c7 100644 --- a/crates/fspy/src/unix/syscall_handler/mod.rs +++ b/crates/fspy/src/unix/syscall_handler/mod.rs @@ -1,5 +1,6 @@ mod execve; mod getdents; +mod mutate; mod open; mod stat; @@ -21,6 +22,15 @@ use crate::arena::PathAccessArena; const PATH_MAX: usize = libc::PATH_MAX as usize; +/// Whether a path is a directory right now. +/// +/// Read before the syscall runs, so this is existing filesystem state and not +/// the call's result. A path that cannot be stated is reported as a file, which +/// only costs the subtree re-attribution a directory rename would have got. +fn is_existing_dir(path: &Path) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|metadata| metadata.is_dir()) +} + #[derive(Debug)] pub struct SyscallHandler { arena: PathAccessArena, @@ -38,6 +48,33 @@ impl SyscallHandler { self.arena } + /// Reads a syscall's path argument and makes it absolute. + /// + /// Returns `None` when the path does not fit in `PATH_MAX`, matching the + /// rest of this handler: such a path is dropped rather than truncated. + /// + /// The result is owned because callers that handle two paths at once, like + /// rename, cannot both borrow the single read buffer. + fn resolve_path( + &mut self, + caller: Caller, + dir_fd: Fd, + path_ptr: CStrPtr, + ) -> io::Result> { + let Some(path_len) = path_ptr.read(caller, &mut self.path_read_buf)? else { + return Ok(None); + }; + let path = Path::new(OsStr::from_bytes(&self.path_read_buf[..path_len])); + if path.is_absolute() { + return Ok(Some(path.to_path_buf())); + } + let mut resolved_path = PathBuf::from(dir_fd.get_path(caller)?); + if !nix::NixPath::is_empty(path) { + resolved_path.push(path); + } + Ok(Some(resolved_path)) + } + fn handle_open( &mut self, caller: Caller, @@ -82,6 +119,66 @@ impl SyscallHandler { Ok(()) } + /// Records a rename as a delete of the source and a write of the + /// destination. + /// + /// Both halves come from the call's arguments. Whether the source is a + /// directory is read from the filesystem before the syscall runs, which is + /// current state rather than the call's outcome, and it matters because a + /// directory rename re-attributes every write recorded under the old + /// subtree. + /// + /// `exchange` covers `RENAME_EXCHANGE`, where the source is replaced by the + /// destination instead of disappearing, so it is written rather than deleted. + fn handle_rename( + &mut self, + caller: Caller, + (old_dir_fd, old_path_ptr): (Fd, CStrPtr), + (new_dir_fd, new_path_ptr): (Fd, CStrPtr), + exchange: bool, + ) -> io::Result<()> { + let Some(source) = self.resolve_path(caller, old_dir_fd, old_path_ptr)? else { + return Ok(()); + }; + let Some(dest) = self.resolve_path(caller, new_dir_fd, new_path_ptr)? else { + return Ok(()); + }; + + let dir_flag = + if is_existing_dir(&source) { AccessMode::IS_DIR } else { AccessMode::empty() }; + + let source_mode = if exchange { + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag + } else { + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag + }; + self.arena.add(PathAccess { mode: source_mode, path: source.as_os_str().into() }); + self.arena.add(PathAccess { + mode: AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + path: dest.as_os_str().into(), + }); + Ok(()) + } + + /// Records a delete from the call's arguments. + fn handle_delete( + &mut self, + caller: Caller, + dir_fd: Fd, + path_ptr: CStrPtr, + is_dir: bool, + ) -> io::Result<()> { + let Some(path) = self.resolve_path(caller, dir_fd, path_ptr)? else { + return Ok(()); + }; + let dir_flag = if is_dir { AccessMode::IS_DIR } else { AccessMode::empty() }; + self.arena.add(PathAccess { + mode: AccessMode::DELETED | dir_flag, + path: path.as_os_str().into(), + }); + Ok(()) + } + fn handle_open_dir(&mut self, caller: Caller, fd: Fd) -> io::Result<()> { let path = fd.get_path(caller)?; self.arena.add(PathAccess { @@ -112,6 +209,13 @@ impl_handler!( faccessat, faccessat2, + #[cfg(target_arch = "x86_64")] rename, + renameat, + renameat2, + #[cfg(target_arch = "x86_64")] unlink, + unlinkat, + #[cfg(target_arch = "x86_64")] rmdir, + execve, execveat, ); diff --git a/crates/fspy/src/unix/syscall_handler/mutate.rs b/crates/fspy/src/unix/syscall_handler/mutate.rs new file mode 100644 index 000000000..21b20e586 --- /dev/null +++ b/crates/fspy/src/unix/syscall_handler/mutate.rs @@ -0,0 +1,64 @@ +//! Rename and delete, the syscalls that move a task's output into place. +//! +//! Without these a build that stages into a temporary directory and renames it +//! over the real one looks like it wrote nothing that still exists, so its +//! archive comes out empty. The preload backend intercepts the libc wrappers; +//! this backend has to see the syscalls themselves, which is what makes the two +//! agree on musl. +//! +//! Every flag here is derived from the call's arguments. This supervisor +//! responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` and is notified before the +//! syscall runs, so it never learns whether the call succeeded. + +use std::{ffi::c_int, io}; + +use fspy_seccomp_unotify::supervisor::handler::arg::{CStrPtr, Caller, Fd}; + +use super::SyscallHandler; + +impl SyscallHandler { + #[cfg(target_arch = "x86_64")] + pub(super) fn rename( + &mut self, + caller: Caller, + (old_path, new_path): (CStrPtr, CStrPtr), + ) -> io::Result<()> { + self.handle_rename(caller, (Fd::cwd(), old_path), (Fd::cwd(), new_path), false) + } + + pub(super) fn renameat( + &mut self, + caller: Caller, + (old_dir_fd, old_path, new_dir_fd, new_path): (Fd, CStrPtr, Fd, CStrPtr), + ) -> io::Result<()> { + self.handle_rename(caller, (old_dir_fd, old_path), (new_dir_fd, new_path), false) + } + + pub(super) fn renameat2( + &mut self, + caller: Caller, + (old_dir_fd, old_path, new_dir_fd, new_path, flags): (Fd, CStrPtr, Fd, CStrPtr, c_int), + ) -> io::Result<()> { + let exchange = flags & c_int::try_from(libc::RENAME_EXCHANGE).unwrap_or(0) != 0; + self.handle_rename(caller, (old_dir_fd, old_path), (new_dir_fd, new_path), exchange) + } + + #[cfg(target_arch = "x86_64")] + pub(super) fn unlink(&mut self, caller: Caller, (path,): (CStrPtr,)) -> io::Result<()> { + self.handle_delete(caller, Fd::cwd(), path, false) + } + + pub(super) fn unlinkat( + &mut self, + caller: Caller, + (dir_fd, path, flags): (Fd, CStrPtr, c_int), + ) -> io::Result<()> { + let is_dir = flags & libc::AT_REMOVEDIR != 0; + self.handle_delete(caller, dir_fd, path, is_dir) + } + + #[cfg(target_arch = "x86_64")] + pub(super) fn rmdir(&mut self, caller: Caller, (path,): (CStrPtr,)) -> io::Result<()> { + self.handle_delete(caller, Fd::cwd(), path, true) + } +} diff --git a/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs b/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs index fa9dc305d..85baab3bd 100644 --- a/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs +++ b/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs @@ -227,3 +227,22 @@ impl FromNotify for (T1, T2, T3, T4, T5) +{ + fn from_notify(notif: &seccomp_notif) -> io::Result { + Ok(( + T1::from_syscall_arg(notif.data.args[0])?, + T2::from_syscall_arg(notif.data.args[1])?, + T3::from_syscall_arg(notif.data.args[2])?, + T4::from_syscall_arg(notif.data.args[3])?, + T5::from_syscall_arg(notif.data.args[4])?, + )) + } +} From 47250b4501a3d6a3832085729babafce62675924 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Mon, 27 Jul 2026 09:54:21 +0800 Subject: [PATCH 20/21] docs: record the two open tracking issues found on emdash Neither is a regression from this branch, and neither is fixed here. The `--parallel` hit-rate loss and the three tasks that never hit both trace to pnpm self-linking a workspace package, so `packages/core/dist` and `node_modules/emdash/dist` are one directory under two names. The own-derived-state rule attributes by path and only recognises the first. Records what is measured, what is still hypothesis, and the reproduction for each, including one hypothesis already ruled out: the files appearing mid-run are core's own output, not a second writer. Co-authored-by: Claude Opus 5 --- INVESTIGATE.md | 193 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 INVESTIGATE.md diff --git a/INVESTIGATE.md b/INVESTIGATE.md new file mode 100644 index 000000000..92eeed245 --- /dev/null +++ b/INVESTIGATE.md @@ -0,0 +1,193 @@ +# Things to investigate + +Found while verifying automatic input/output tracking (#571) against +[emdash](https://github.com/emdash-cms/emdash): 20 packages, 25 build tasks, +mostly plain `tsdown`. + +Neither item is a regression from #571 — both reproduce as described with that +branch's rules in place, and #2b is a case the rules were meant to cover but +attribute by the wrong path. + +Both trace to the same underlying fact. pnpm links every workspace package into +`node_modules/`, and `packages/core` is the package named `emdash`: + +``` +node_modules/emdash -> ../packages/core +``` + +So `packages/core/dist` and `node_modules/emdash/dist` are the same directory +under two names. Tracking attributes accesses by path, so a write recorded +against one name is not recognised as touching the other. + +**Environment.** `vp 0.0.0-commit.88ab4592d55342c11a070c47e8fc7205f675fda3` +(preview of voidzero-dev/vite-plus#2260, carrying vite-task #571), emdash branch +`agent/vite-plus-package-build-cache`. Local: macOS 26.5.1, M4 Pro, 12 cores. +CI: `ubuntu-latest`, 4 vCPU. + +--- + +## 1. `--parallel` appears to fingerprint partially written dependency output + +**Severity:** correctness-adjacent, not just a hit-rate loss. A stored cache entry +describes a dependency directory that was mid-write, so the entry is keyed on +torn state. It does not currently produce a wrong tree, because the next run +detects the difference and reruns — but it is a cache entry that never should +have been stored. + +### Observation + +With the default concurrency limit (4), the warm run is a full hit: + +``` +vp run: 25/25 cache hit (100%) +``` + +With `--parallel` (unlimited concurrency), the warm run reproducibly loses 7 +tasks. Three consecutive cold+warm pairs, each after `rm -rf +node_modules/.vite/task-cache`: + +| run | warm result | +| --- | --------------- | +| 1 | 18/25 cache hit | +| 2 | 18/25 cache hit | +| 3 | 18/25 cache hit | + +An earlier pass gave 19/25, so the count is not perfectly fixed. + +### Reproduction + +```bash +cd emdash +rm -rf node_modules/.vite/task-cache +pnpm exec vp run --cache --parallel --filter '{./packages/**}' build # cold +pnpm exec vp run --cache --parallel --filter '{./packages/**}' build # warm +``` + +### Evidence + +Every miss names a file _appearing_ in `node_modules/emdash/dist`, i.e. in +`packages/core`'s output: + +``` +~/packages/cloudflare$ tsdown ○ cache miss: 'index.d.mts' added in 'node_modules/emdash/dist/seed' +~/packages/workerd$ tsdown ○ cache miss: 'index.mjs' added in 'node_modules/emdash/dist' +~/packages/plugins/sandboxed-test$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' +~/packages/plugins/webhook-notifier$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' +~/packages/plugins/audit-log$ node ...build ○ cache miss: 'plugin-types.d.mts' added in 'node_modules/emdash/dist' +~/packages/plugins/atproto$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' +~/packages/plugins/marketplace-test$ node ...build ○ cache miss: 'plugin-types.d.mts' added in 'node_modules/emdash/dist' +``` + +The readers are all consumers of `emdash`. The writer is `packages/core#build`. +Which file is named varies between runs — `index.d.mts`, `plugin-types.mjs`, +`middleware.d.mts`, `request-context.d.mts` — which is what a race looks like. + +### Hypothesis (not yet confirmed) + +`added in` means the directory had **more** entries at fingerprint time than when +the entry was stored. So the corruption is on the **cold** run: consumers +fingerprinted `node_modules/emdash/dist` while `packages/core#build` was still +writing into it, storing an entry that describes an incomplete directory. The +warm run then sees the finished directory and correctly reports a difference. + +That implies the consumers were running concurrently with `packages/core#build`, +which raises the open question below. + +### Open questions + +- Does `--parallel` still respect dependency order? Concurrency is described as + per graph level (`vite_task_plan/src/plan.rs:742`), which should keep + `packages/core#build` in an earlier level than its consumers. If ordering holds, + consumers should never observe a partial `dist`, and the hypothesis above is + wrong — so what else explains `added in`? +- A single writer is confirmed, so this is not two tasks writing the same + directory. Every file named in the misses is `packages/core`'s own output: + `plugin-types.mjs`, `plugin-types.d.mts` and `index.mjs` all live in + `packages/core/dist/`, produced by core's own `tsdown` entry points. The + separate `@emdash-cms/plugin-types` package builds into its own `dist` and does + not write here. That leaves timing, not ownership, as the thing to explain. +- Should a directory fingerprint taken while another task in the same run is + writing to that directory be storable at all, or should the run decline to cache + the reader? + +### Suggested next step + +Re-run with `VITE_TASK_DEBUG_TRACKING=1` on the cold `--parallel` run and compare +the recorded write timestamps for `packages/core#build` against the reads from +consumers, to establish whether the two actually overlap in time. That settles +ordering versus second-writer before any fix is designed. + +--- + +## 2. Three tasks never hit on emdash, even with the default limit + +Steady state on CI with the concurrency limit at its default, and with no change +touching any package: + +``` +vp run: 22/25 cache hit (88%), 115.61s saved +``` + +The same three tasks miss every run: + +``` +~/packages/registry-lexicons$ pnpm run build:lexicons ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/registry-lexicons$ pnpm run build:types ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/core$ tsdown ○ cache miss: 'index.d.mts' removed from 'node_modules/emdash/dist' +``` + +### 2a. `registry-lexicons` reads pnpm's workspace state file + +Both tasks run through `pnpm run ...`, so the `pnpm` process itself reads +`node_modules/.pnpm-workspace-state-v1.json`. pnpm rewrites that file on install, +so its content cannot survive a fresh CI checkout, and the fingerprint can never +settle across machines. + +This is the case upstream documents under +[When To Add Manual Config](https://viteplus.dev/guide/automatic-data-tracking#when-to-add-manual-config), +and the immediate workaround is a manual `input` exclusion in emdash. + +**Worth deciding upstream:** whether this belongs in the same always-ignored set +as `node_modules` and `.git`. It is a package manager's own bookkeeping, it is +never an authored source, and any task invoked via `pnpm run` will read it — so +every pnpm workspace that wraps a task in `pnpm run` hits this. Note the file +already lives under `node_modules`, so the existing rule does not catch it: that +rule only resolves read-then-write overlaps, and this is a pure read, which is +legitimately an input. + +### 2b. `packages/core` reads its own output through the `node_modules` alias + +`packages/core` is the package named `emdash`, so pnpm self-links it and +`tsdown` resolves the package's own entry points through +`node_modules/emdash/dist/...`. The task therefore reads its own output. + +#571 added a rule for exactly this shape — a gitignored path underneath a +directory the task wrote into is the task's own derived state, not an input — but +it attributes by path. The write is recorded at `packages/core/dist/index.d.mts` +and the read at `node_modules/emdash/dist/index.d.mts`. The read path is not +lexically under `packages/core/`, so the rule does not fire. + +On a fresh checkout the directory does not exist yet when the task starts, which +is why the miss reads `removed from` rather than `modified`. + +**Fix direction to evaluate:** resolve symlinks when attributing an access, so +both names collapse to one identity before classification. Open questions: + +- Cost. This would mean a `realpath`-style resolution per tracked path, on a hot + path that currently does string work only. Resolving just the + `node_modules/` prefix may be enough and much cheaper. +- Where to resolve. Doing it in the tracer changes what every backend reports and + loses the name the task actually used, which is the name that appears in cache + miss messages. Doing it in classification keeps reporting intact. +- Whether the workspace already knows the answer. `vite_workspace` maps package + names to directories, so `node_modules/emdash` -> `packages/core` may be + derivable without touching the filesystem at all. This looks like the cheapest + option and should be checked first. +- Correctness limit: this fixes the alias case, not hard links, and not two + distinct symlinks into the same tree. + +### Note on measurement + +These three misses cost real time but are not why the cache is useful here: the +same run still saved 115.61s. Fixing 2a and 2b would take emdash from 22/25 to +25/25 on CI, matching what a warm local run already achieves. From e7f53a763df79b8079c21b4ee8207988224f0754 Mon Sep 17 00:00:00 2001 From: wan9chi Date: Mon, 27 Jul 2026 10:25:13 +0800 Subject: [PATCH 21/21] docs: diagnose task cache instability and propose fixes Rewrites the investigation notes now that both causes are measured rather than suspected, and corrects the earlier `--parallel` entry. Two causes, both fixable here. A workspace package reachable under two names via its node_modules self-link, so the own-derived-state rule from #571 compares path strings and misses its own output. And a package manager's workspace-state file, which carries a wall-clock timestamp and absolute paths, so any task wrapping `pnpm run` is uncacheable across machines and also defeats cache portability across workspace roots. `--parallel` was recorded as a possible correctness bug. It is documented as running without dependency ordering, and timestamped task starts confirm it behaves as documented, so that entry is withdrawn. Co-authored-by: Claude Opus 5 --- INVESTIGATE.md | 416 +++++++++++++++++++++++++++++++------------------ 1 file changed, 262 insertions(+), 154 deletions(-) diff --git a/INVESTIGATE.md b/INVESTIGATE.md index 92eeed245..f01127486 100644 --- a/INVESTIGATE.md +++ b/INVESTIGATE.md @@ -1,193 +1,301 @@ -# Things to investigate +# Task cache instability: findings and proposed fixes -Found while verifying automatic input/output tracking (#571) against -[emdash](https://github.com/emdash-cms/emdash): 20 packages, 25 build tasks, -mostly plain `tsdown`. +Investigation of everything that makes a task unstable or uncacheable, run +against [emdash](https://github.com/emdash-cms/emdash) (20 packages, 25 build +tasks, mostly plain `tsdown`) with the automatic tracking rules from #571. -Neither item is a regression from #571 — both reproduce as described with that -branch's rules in place, and #2b is a case the rules were meant to cover but -attribute by the wrong path. +None of the proposals below are implemented or validated yet. Diagnoses are +marked **confirmed** (directly measured) or **hypothesis**. -Both trace to the same underlying fact. pnpm links every workspace package into -`node_modules/`, and `packages/core` is the package named `emdash`: +**Environment.** `vp 0.0.0-commit.88ab4592d55342c11a070c47e8fc7205f675fda3` +(preview of voidzero-dev/vite-plus#2260, carrying #571), emdash branch +`agent/vite-plus-package-build-cache`. Local: macOS 26.5.1, M4 Pro, 12 cores. +CI: `ubuntu-latest`, 4 vCPU. + +## Scope of the problem + +Two things are worth separating, because they have very different weight. + +**Repeat-run stability is already perfect.** Five consecutive warm runs with no +intervening change, all 25/25: + +| run | 1 | 2 | 3 | 4 | 5 | +| ---- | ----- | ----- | ----- | ----- | ----- | +| hits | 25/25 | 25/25 | 25/25 | 25/25 | 25/25 | + +A no-op `pnpm install` also preserves 25/25 — it leaves +`node_modules/.pnpm-workspace-state-v1.json` byte-identical. + +**Instability only appears across an environment transition** — a fresh checkout, +a real install, or a different machine. On CI, steady state is 22/25, and the same +three tasks miss every single run: ``` -node_modules/emdash -> ../packages/core +~/packages/registry-lexicons$ pnpm run build:lexicons ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/registry-lexicons$ pnpm run build:types ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/core$ tsdown ○ cache miss: 'index.d.mts' removed from 'node_modules/emdash/dist' ``` -So `packages/core/dist` and `node_modules/emdash/dist` are the same directory -under two names. Tracking attributes accesses by path, so a write recorded -against one name is not recognised as touching the other. +Two distinct causes, both fixable in vite-task. Neither is a regression from #571; +cause A is a case #571's rules were meant to cover but attribute by the wrong +path. -**Environment.** `vp 0.0.0-commit.88ab4592d55342c11a070c47e8fc7205f675fda3` -(preview of voidzero-dev/vite-plus#2260, carrying vite-task #571), emdash branch -`agent/vite-plus-package-build-cache`. Local: macOS 26.5.1, M4 Pro, 12 cores. -CI: `ubuntu-latest`, 4 vCPU. +| | cause | tasks | fixable in vite-task | +| --- | ---------------------------------------------------------------------------- | ----- | -------------------- | +| A | workspace package reachable under two names via its `node_modules` self-link | 1 | yes | +| B | package manager bookkeeping file read by any `pnpm run` task | 2 | yes | --- -## 1. `--parallel` appears to fingerprint partially written dependency output +## Cause A — a workspace package's output has two names -**Severity:** correctness-adjacent, not just a hit-rate loss. A stored cache entry -describes a dependency directory that was mid-write, so the entry is keyed on -torn state. It does not currently produce a wrong tree, because the next run -detects the difference and reruns — but it is a cache entry that never should -have been stored. +**Confirmed.** pnpm links every workspace package into `node_modules/`, and +`packages/core` _is_ the package named `emdash`: -### Observation +``` +node_modules/emdash -> ../packages/core +``` -With the default concurrency limit (4), the warm run is a full hit: +So `packages/core/dist` and `node_modules/emdash/dist` are one directory. Tracing +`packages/core`'s own `tsdown` (119,037 accesses) shows it writing and reading the +same file under different names: ``` -vp run: 25/25 cache hit (100%) +WRITE packages/core/dist/index.d.mts AccessMode(WRITE | CREATE | TRUNCATE) +READ node_modules/emdash/dist/index.d.mts AccessMode(READ) ``` -With `--parallel` (unlimited concurrency), the warm run reproducibly loses 7 -tasks. Three consecutive cold+warm pairs, each after `rm -rf -node_modules/.vite/task-cache`: +The task consumes its own output because it resolves its own package through the +self-link. #571 has a rule for exactly this shape — a gitignored path underneath a +directory the task wrote into is the task's own derived state, not an input — and +`dist/` is gitignored at emdash's root. The rule does not fire because it compares +path strings: `node_modules/emdash/dist` is not lexically under `packages/core/`, +so the read is recorded as an input. + +On a fresh checkout that path does not exist when the task starts, which is why +the miss says `removed from` rather than `modified`. The task can therefore never +hit on a clean machine. + +**Blast radius.** Any workspace package whose build resolves its own package by +name. That is normal for packages with `exports` self-references, and it is not +emdash-specific. + +### Proposed fixes + +**A1 — canonicalise every tracked path (not recommended).** Resolve symlinks for +all accesses before classification. Simple rule, but 119k accesses for one task +means a `realpath` per path on a hot path that is currently pure string work. Worse, +pnpm's `node_modules/` entries point into `node_modules/.pnpm/...`, and under +some store layouts resolution can land outside the workspace root — where +`strip_prefix(workspace_root)` (`cache_update.rs:315`) silently drops the path, so +real dependency inputs would stop being tracked. Rejecting this because the failure +mode is a _lost_ input, which is worse than a missed cache. + +**A2 — normalise workspace-package aliases using the package graph (recommended).** +vite-task already knows every workspace package's name and directory. Build a map +of ` -> ` once per run and rewrite any tracked path matching +`**/node_modules//...` to `/...`. Pure string work, no syscalls, +and bounded by the number of workspace packages rather than the number of accesses. + +After rewriting, core's read becomes `packages/core/dist/index.d.mts`, which _is_ +under a directory it wrote into and _is_ gitignored, so the existing +own-derived-state rule fires unchanged and the task hits. + +This also improves consumers: `packages/cloudflare`'s input becomes +`packages/core/dist/...` instead of `node_modules/emdash/dist/...`. Same file, but +the canonical name, so cache miss messages name the real path and two tasks +referring to one file agree on its identity. + +Costs and open points: + +- Must handle scoped names (`node_modules/@emdash-cms/plugin-cli`), which are two + path segments, and nested `packages/x/node_modules/`. Both were observed. +- `ClassifyContext` (`classify.rs:44`) currently has only `workspace_root`, the two + infer flags and `is_gitignored`, so the map needs threading into `observe_fspy` + (`cache_update.rs:194`). That is new plumbing but no new data source. +- Changes recorded input paths, so it needs a `CACHE_SCHEMA_VERSION` bump. + +**A3 — narrow variant of A2, self-links only.** Rewrite only when the alias +resolves to _this task's own_ package directory. `SpawnFingerprint` already carries +`cwd: RelativePathBuf` (`cache_metadata.rs:90`, currently `pub(crate)`), so this +needs almost no plumbing — just widened visibility — and one `read_link` per +distinct alias. + +Fixes the reported miss with the smallest possible behaviour change, and needs no +schema bump if consumers' paths are left alone. Does not give consumers the +canonical-name benefit. + +**Recommendation: A2**, with A3 as the low-risk fallback if the plumbing or the +schema bump is unwelcome this cycle. A2 is a rule about identity that happens to +fix a caching bug; A3 patches the one symptom. -| run | warm result | -| --- | --------------- | -| 1 | 18/25 cache hit | -| 2 | 18/25 cache hit | -| 3 | 18/25 cache hit | +--- -An earlier pass gave 19/25, so the count is not perfectly fixed. +## Cause B — `pnpm run` makes a task uncacheable across machines -### Reproduction +**Confirmed** that the file is tracked as an input to those two tasks and that its +content is unstable. **Hypothesis** that `pnpm` itself is the reader: tracing +`pnpm run build:lexicons` directly under fspy aborts with SIGABRT before it gets +that far, which is the pre-existing pnpm SEA bug (`Assertion failed: (magic) == +(kMagic)`) recorded against `main`. The attribution rests on vite-task naming the +file as the changed input for exactly the two tasks that spawn pnpm, and on no +other emdash task touching it. -```bash -cd emdash -rm -rf node_modules/.vite/task-cache -pnpm exec vp run --cache --parallel --filter '{./packages/**}' build # cold -pnpm exec vp run --cache --parallel --filter '{./packages/**}' build # warm -``` +`registry-lexicons` is the only package whose build wraps the package manager: -### Evidence +```json +"build": "pnpm run build:lexicons && pnpm run build:types" +``` -Every miss names a file _appearing_ in `node_modules/emdash/dist`, i.e. in -`packages/core`'s output: +That file cannot be fingerprinted stably, by construction: ``` -~/packages/cloudflare$ tsdown ○ cache miss: 'index.d.mts' added in 'node_modules/emdash/dist/seed' -~/packages/workerd$ tsdown ○ cache miss: 'index.mjs' added in 'node_modules/emdash/dist' -~/packages/plugins/sandboxed-test$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' -~/packages/plugins/webhook-notifier$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' -~/packages/plugins/audit-log$ node ...build ○ cache miss: 'plugin-types.d.mts' added in 'node_modules/emdash/dist' -~/packages/plugins/atproto$ node ...build ○ cache miss: 'plugin-types.mjs' added in 'node_modules/emdash/dist' -~/packages/plugins/marketplace-test$ node ...build ○ cache miss: 'plugin-types.d.mts' added in 'node_modules/emdash/dist' +lastValidatedTimestamp: 1785115273801 <- wall clock +projects: { "/Users/chwang/code/emdash": {...}, <- absolute paths + "/Users/chwang/code/emdash/apps/aggregator": {...}, ... } ``` -The readers are all consumers of `emdash`. The writer is `packages/core#build`. -Which file is named varies between runs — `index.d.mts`, `plugin-types.mjs`, -`middleware.d.mts`, `request-context.d.mts` — which is what a race looks like. - -### Hypothesis (not yet confirmed) - -`added in` means the directory had **more** entries at fingerprint time than when -the entry was stored. So the corruption is on the **cold** run: consumers -fingerprinted `node_modules/emdash/dist` while `packages/core#build` was still -writing into it, storing an entry that describes an incomplete directory. The -warm run then sees the finished directory and correctly reports a difference. - -That implies the consumers were running concurrently with `packages/core#build`, -which raises the open question below. - -### Open questions - -- Does `--parallel` still respect dependency order? Concurrency is described as - per graph level (`vite_task_plan/src/plan.rs:742`), which should keep - `packages/core#build` in an earlier level than its consumers. If ordering holds, - consumers should never observe a partial `dist`, and the hypothesis above is - wrong — so what else explains `added in`? -- A single writer is confirmed, so this is not two tasks writing the same - directory. Every file named in the misses is `packages/core`'s own output: - `plugin-types.mjs`, `plugin-types.d.mts` and `index.mjs` all live in - `packages/core/dist/`, produced by core's own `tsdown` entry points. The - separate `@emdash-cms/plugin-types` package builds into its own `dist` and does - not write here. That leaves timing, not ownership, as the thing to explain. -- Should a directory fingerprint taken while another task in the same run is - writing to that directory be storable at all, or should the run decline to cache - the reader? - -### Suggested next step - -Re-run with `VITE_TASK_DEBUG_TRACKING=1` on the cold `--parallel` run and compare -the recorded write timestamps for `packages/core#build` against the reads from -consumers, to establish whether the two actually overlap in time. That settles -ordering versus second-writer before any fix is designed. +The timestamp changes on any revalidating install. The absolute paths differ +between machines and between a local checkout and `/home/runner/work/emdash/emdash`, +so this **also defeats cache portability across workspace roots**, which vite-task +otherwise supports and tests. ---- +It is not caught by #571's always-ignored set. That set (`node_modules`, `.git`) +only resolves read-then-write overlaps; this is a pure read, and pure reads under +`node_modules` are legitimate inputs — that is how dependency code is tracked. + +**Blast radius.** Every task in every pnpm workspace that wraps a script in +`pnpm run`, which is a very common pattern. Each such task is permanently +uncacheable on CI. This is the more widely damaging of the two causes even though +it looks like the more niche one. -## 2. Three tasks never hit on emdash, even with the default limit +### Proposed fixes -Steady state on CI with the concurrency limit at its default, and with no change -touching any package: +**B1 — ignore package-manager bookkeeping files as inputs (recommended).** Keep a +small known list of files that are a package manager's own state and never an +authored source or a meaningful dependency. Present in the emdash checkout and +verified to exist: ``` -vp run: 22/25 cache hit (88%), 115.61s saved +node_modules/.pnpm-workspace-state-v1.json <- the one causing the miss +node_modules/.modules.yaml +node_modules/.pnpm/lock.yaml ``` -The same three tasks miss every run: +Equivalents for other managers, named from their documented layouts and not +observed here, so worth confirming before adding: ``` -~/packages/registry-lexicons$ pnpm run build:lexicons ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified -~/packages/registry-lexicons$ pnpm run build:types ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified -~/packages/core$ tsdown ○ cache miss: 'index.d.mts' removed from 'node_modules/emdash/dist' +node_modules/.package-lock.json (npm) +node_modules/.yarn-state.yml (yarn, node_modules linker) +``` + +Applied to reads, not just overlaps. Low risk: these files describe how +`node_modules` was produced, and the thing that actually matters — the installed +package contents — is already tracked directly. + +`node_modules/.package-map.json` also exists in emdash but is a weaker candidate: +it contains no absolute paths, so it does not break portability the way the +workspace-state file does. Leave it out until something is shown to be destabilised +by it. + +Worth pairing with a rule of thumb for the list: a file belongs on it only if its +content is derived from the lockfile plus the local environment, so that anything +it could tell a task is either already tracked or is machine identity. + +**B2 — ignore by shape rather than by name.** Treat `node_modules/.` as +manager state. Simpler and needs no list maintenance, but over-broad: +`node_modules/.bin/*` lives there and is a real input (three `.bin` reads appear in +core's trace), and `node_modules/.vite/` is vite's own cache directory. + +**B3 — leave it to users.** Document a manual `input` exclusion, which is what +[When To Add Manual Config](https://viteplus.dev/guide/automatic-data-tracking#when-to-add-manual-config) +already implies. Zero risk, but every pnpm monorepo has to rediscover this, and the +symptom — one task that never hits, only on CI — is expensive to diagnose. The +whole point of automatic tracking is not needing this. + +**Recommendation: B1.** B2's over-reach is a real regression risk for `.bin`, and +B3 pushes a general defect onto every user. + +On the emdash side there is also a one-line workaround worth noting: the package's +own `prepublishOnly` already uses `node --run build`, and switching `build` to +`node --run` avoids spawning pnpm at all. That is a good local fix but does not +help the general case. + +--- + +## Not a cause: `--parallel` + +I previously recorded `--parallel` losing cache hits (18/25, reproducible 3/3) as a +possible correctness bug. **That was wrong, and this corrects it.** `vp run --help` +is explicit: + +``` +--parallel Run tasks without dependency ordering. Sets concurrency to + unlimited unless `--concurrency-limit` is also specified ``` -### 2a. `registry-lexicons` reads pnpm's workspace state file - -Both tasks run through `pnpm run ...`, so the `pnpm` process itself reads -`node_modules/.pnpm-workspace-state-v1.json`. pnpm rewrites that file on install, -so its content cannot survive a fresh CI checkout, and the fingerprint can never -settle across machines. - -This is the case upstream documents under -[When To Add Manual Config](https://viteplus.dev/guide/automatic-data-tracking#when-to-add-manual-config), -and the immediate workaround is a manual `input` exclusion in emdash. - -**Worth deciding upstream:** whether this belongs in the same always-ignored set -as `node_modules` and `.git`. It is a package manager's own bookkeeping, it is -never an authored source, and any task invoked via `pnpm run` will read it — so -every pnpm workspace that wraps a task in `pnpm run` hits this. Note the file -already lives under `node_modules`, so the existing rule does not catch it: that -rule only resolves read-then-write overlaps, and this is a pure read, which is -legitimately an input. - -### 2b. `packages/core` reads its own output through the `node_modules` alias - -`packages/core` is the package named `emdash`, so pnpm self-links it and -`tsdown` resolves the package's own entry points through -`node_modules/emdash/dist/...`. The task therefore reads its own output. - -#571 added a rule for exactly this shape — a gitignored path underneath a -directory the task wrote into is the task's own derived state, not an input — but -it attributes by path. The write is recorded at `packages/core/dist/index.d.mts` -and the read at `node_modules/emdash/dist/index.d.mts`. The read path is not -lexically under `packages/core/`, so the rule does not fire. - -On a fresh checkout the directory does not exist yet when the task starts, which -is why the miss reads `removed from` rather than `modified`. - -**Fix direction to evaluate:** resolve symlinks when attributing an access, so -both names collapse to one identity before classification. Open questions: - -- Cost. This would mean a `realpath`-style resolution per tracked path, on a hot - path that currently does string work only. Resolving just the - `node_modules/` prefix may be enough and much cheaper. -- Where to resolve. Doing it in the tracer changes what every backend reports and - loses the name the task actually used, which is the name that appears in cache - miss messages. Doing it in classification keeps reporting intact. -- Whether the workspace already knows the answer. `vite_workspace` maps package - names to directories, so `node_modules/emdash` -> `packages/core` may be - derivable without touching the filesystem at all. This looks like the cheapest - option and should be checked first. -- Correctness limit: this fixes the alias case, not hard links, and not two - distinct symlinks into the same tree. - -### Note on measurement - -These three misses cost real time but are not why the cache is useful here: the -same run still saved 115.61s. Fixing 2a and 2b would take emdash from 22/25 to -25/25 on CI, matching what a warm local run already achieves. +Timestamped task starts confirm the documented behaviour rather than a bug: + +| | `packages/core` starts | consumers start | +| ----------------- | ---------------------- | -------------------------------------------- | +| default limit (4) | 444.832 | 470.767 – 475.772 (after core finishes) | +| `--parallel` | 382.231 | 382.144 – 382.466 (cloudflare _before_ core) | + +With ordering discarded, consumers fingerprint `node_modules/emdash/dist` while +core is still writing it, so the cold run stores entries describing a partially +written directory and the warm run correctly reports `added in`. Using `--parallel` +on a workspace where packages consume each other's build output is simply the wrong +flag. Nothing to fix. + +One optional hardening, low priority: when a task's inferred inputs overlap another +task's inferred outputs _within the same run_, the entry is built on unordered +state. vite-task could decline to cache that task, or warn. It would convert a +silently useless cache entry into a visible explanation. Worth it only if +`--parallel` misuse turns out to be common. + +## Not a cause: paths outside the workspace root + +`strip_prefix(workspace_root)` (`cache_update.rs:315`) drops every access outside +the workspace, so the 388 `/var/folders` temp accesses and the `/opt/homebrew` +reads in core's trace cannot destabilise anything. + +The same rule means the toolchain is not fingerprinted: `node` lives under +`~/.local/share/mise/...`, so a Node or `tsdown` upgrade does not invalidate a +cached task. That is a deliberate portability tradeoff, not instability, but it is +worth being explicit that cache keys describe the workspace and not the machine. + +## Checked and clean + +For `packages/core`'s build, none of the usual suspects appear: no `.tsbuildinfo`, +no `node_modules/.vite/` cache reads, no `pnpm-lock.yaml` read, no log files. The +only `node_modules/.bin` accesses are three plain reads of shims. It also touches +none of the manager state files from cause B — zero accesses to +`.pnpm-workspace-state-v1.json`, `.modules.yaml`, `.package-map.json`, +`.pnpm/lock.yaml`, `.vite/` or `.vite-temp/` — which is consistent with only the +two pnpm-spawning tasks missing. + +`node_modules/.vite-temp/` deserves a note because it looks dangerous and is not: +vitest writes `vitest.config.js.timestamp-.mjs` files there, so its directory +listing changes every run. #571 classifies those as write-first outputs and the +directory is empty at archive time, and the e2e fixture covering it passes. It is +not implicated in emdash's builds. + +So for this workspace the instability really is just causes A and B, not a long +tail. + +That is a statement about emdash, not a general audit. A workspace using +`tsc --build` would add `.tsbuildinfo`, which embeds absolute paths and timestamps +and would likely need the same treatment as cause B. + +## Suggested order of work + +1. **B1** — smallest change, widest benefit, affects every pnpm monorepo. +2. **A2** — larger, needs plumbing and a schema bump, but fixes a real gap in + #571's own rule and makes path identity consistent. +3. Optional `--parallel` diagnostic, only if misuse is seen in the wild. + +Both A and B should be reproduced as e2e fixtures first, since both are currently +only observed through emdash. A fixture for A needs a package whose build resolves +its own name through a `node_modules` self-link; for B, a task that shells out to +`pnpm run` with a workspace state file present.