Skip to content

Commit b5529aa

Browse files
branchseerclaude
andcommitted
fix(cache): apply input negatives to reads only, not writes
Output writes were being dropped before archiving when paths matched an input-side negative glob. `TrackedPathAccesses::from_raw` applied one shared negatives list to both reads and writes, so a common config like `input: [{auto:true}, "!dist/**"]` with default auto output left `dist/**` out of the archive and the cache-hit replay failed to restore output files. Move user-configured negative filtering out of `from_raw` (which now only normalises paths and skips `.git`). Reads are filtered by input negatives inside the cache-update path; writes are filtered by output negatives inside `collect_and_archive_outputs` as before. Adds an e2e test `input_negative_does_not_drop_output_writes` to cover the regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2b7f4ca commit b5529aa

5 files changed

Lines changed: 112 additions & 44 deletions

File tree

crates/vite_task/src/session/execute/mod.rs

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,11 @@ struct CacheState<'a> {
290290
/// Captured stdout/stderr for cache replay. Written in place during drain;
291291
/// always present (possibly empty) once we reach the cache-update phase.
292292
std_outputs: Vec<StdOutput>,
293-
/// `Some` iff fspy is enabled (`includes_auto`). Holds the resolved
294-
/// negative globs used by [`TrackedPathAccesses::from_raw`] to filter
295-
/// tracked accesses. `None` means fspy tracking is off for this task.
296-
fspy_negatives: Option<Vec<wax::Glob<'static>>>,
293+
/// Fspy tracking status and pre-resolved input negative globs.
294+
/// `None` means fspy tracking is off for this task. `Some(globs)` means
295+
/// fspy is on; the globs are used to filter inferred input reads (not
296+
/// writes — output negatives are applied separately during archiving).
297+
fspy_input_negatives: Option<Vec<wax::Glob<'static>>>,
297298
}
298299

299300
/// Execute a spawned process with cache-aware lifecycle.
@@ -460,7 +461,7 @@ pub async fn execute_spawn(
460461
metadata,
461462
globbed_inputs,
462463
std_outputs: Vec::new(),
463-
fspy_negatives: fspy,
464+
fspy_input_negatives: fspy,
464465
},
465466
}
466467
}
@@ -472,7 +473,9 @@ pub async fn execute_spawn(
472473

473474
// 5. Derive the arguments for `spawn()` from the mode without consuming it.
474475
let (spawn_stdio, fspy_enabled) = match &mode {
475-
ExecutionMode::Cached { state, .. } => (SpawnStdio::Piped, state.fspy_negatives.is_some()),
476+
ExecutionMode::Cached { state, .. } => {
477+
(SpawnStdio::Piped, state.fspy_input_negatives.is_some())
478+
}
476479
ExecutionMode::Uncached { pipe_writers: Some(_) } => (SpawnStdio::Piped, false),
477480
ExecutionMode::Uncached { pipe_writers: None } => (SpawnStdio::Inherited, false),
478481
};
@@ -558,30 +561,49 @@ pub async fn execute_spawn(
558561
// update are reported but do not affect the exit status we return.
559562
let (cache_update_status, cache_error) = 'cache_update: {
560563
if let ExecutionMode::Cached { state, .. } = mode {
561-
let CacheState { metadata, globbed_inputs, std_outputs, fspy_negatives } = state;
564+
let CacheState { metadata, globbed_inputs, std_outputs, fspy_input_negatives } = state;
562565

563-
// Normalize fspy accesses. `zip` gives `Some` iff fspy was enabled
564-
// (both outcome.path_accesses and fspy_negatives are Some together).
566+
// Normalize fspy accesses. `Some` iff fspy was enabled at spawn time.
567+
// User-configured negatives are applied below, separately for reads
568+
// (input negatives) and writes (output negatives, inside
569+
// `collect_and_archive_outputs`).
565570
let path_accesses = outcome
566571
.path_accesses
567572
.as_ref()
568-
.zip(fspy_negatives.as_deref())
569-
.map(|(raw, negs)| TrackedPathAccesses::from_raw(raw, cache_base_path, negs));
573+
.map(|raw| TrackedPathAccesses::from_raw(raw, cache_base_path));
574+
575+
// Inferred input reads: filter path_reads by input negatives.
576+
// Separate from write filtering (which uses output negatives inside
577+
// `collect_and_archive_outputs`) so users can set `!dist/**` on
578+
// input without accidentally dropping writes to `dist`.
579+
let empty_reads = HashMap::default();
580+
let inferred_reads: HashMap<RelativePathBuf, tracked_accesses::PathRead> =
581+
if let Some(pa) = path_accesses.as_ref()
582+
&& let Some(negatives) = fspy_input_negatives.as_deref()
583+
{
584+
pa.path_reads
585+
.iter()
586+
.filter(|(path, _)| {
587+
!negatives.iter().any(|neg| wax::Program::is_match(neg, path.as_str()))
588+
})
589+
.map(|(path, read)| (path.clone(), *read))
590+
.collect()
591+
} else {
592+
empty_reads
593+
};
570594

571595
let cancelled = fast_fail_token.is_cancelled() || interrupt_token.is_cancelled();
572596
if cancelled {
573597
// Cancelled (Ctrl-C or sibling failure) — result is untrustworthy
574598
(CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::Cancelled), None)
575599
} else if outcome.exit_status.success() {
576600
// Check for read-write overlap: if the task wrote to any file it also
577-
// read, the inputs were modified during execution — don't cache.
578-
// Note: this only checks fspy-inferred reads, not globbed_inputs keys.
579-
// A task that writes to a glob-matched file without reading it causes
580-
// perpetual cache misses (glob detects the hash change) but not a
581-
// correctness bug, so we don't handle that case here.
601+
// read (as an inferred input), the inputs were modified during
602+
// execution — don't cache. Reads excluded by input negatives (or
603+
// when input auto is off) don't count.
582604
if let Some(path) = path_accesses
583605
.as_ref()
584-
.and_then(|pa| pa.path_reads.keys().find(|p| pa.path_writes.contains(*p)))
606+
.and_then(|pa| inferred_reads.keys().find(|p| pa.path_writes.contains(*p)))
585607
{
586608
(
587609
CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::InputModified {
@@ -590,15 +612,14 @@ pub async fn execute_spawn(
590612
None,
591613
)
592614
} else {
593-
// path_reads is empty when inference is disabled (path_accesses is None)
594-
let empty_path_reads = HashMap::default();
595-
let path_reads =
596-
path_accesses.as_ref().map_or(&empty_path_reads, |pa| &pa.path_reads);
597-
598615
// Execution succeeded — attempt to create fingerprint and update cache.
599616
// Paths already in globbed_inputs are skipped: Rule 1 (above) guarantees
600617
// no input modification, so the prerun hash is the correct post-exec hash.
601-
match PostRunFingerprint::create(path_reads, cache_base_path, &globbed_inputs) {
618+
match PostRunFingerprint::create(
619+
&inferred_reads,
620+
cache_base_path,
621+
&globbed_inputs,
622+
) {
602623
Ok(post_run_fingerprint) => {
603624
// Collect output files and create archive
604625
let output_archive = match collect_and_archive_outputs(

crates/vite_task/src/session/execute/tracked_accesses.rs

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ use vite_path::{AbsolutePath, RelativePathBuf};
88

99
use crate::collections::HashMap;
1010

11+
/// User-configured negative globs are NOT applied here. They are applied later,
12+
/// separately for reads (input config) and writes (output config), since those
13+
/// two configs are independent.
14+
1115
/// Path read access info
1216
#[derive(Debug, Clone, Copy)]
1317
pub struct PathRead {
@@ -25,22 +29,19 @@ pub struct TrackedPathAccesses {
2529
}
2630

2731
impl TrackedPathAccesses {
28-
/// Build from fspy's raw iterable by stripping the workspace prefix,
29-
/// normalizing `..` components, and filtering against the negative globs.
30-
pub fn from_raw(
31-
raw: &PathAccessIterable,
32-
workspace_root: &AbsolutePath,
33-
resolved_negatives: &[wax::Glob<'static>],
34-
) -> Self {
32+
/// Build from fspy's raw iterable by stripping the workspace prefix and
33+
/// normalizing `..` components. `.git/*` paths are skipped. User-configured
34+
/// negatives are applied by the caller (see module docs).
35+
pub fn from_raw(raw: &PathAccessIterable, workspace_root: &AbsolutePath) -> Self {
3536
let mut accesses = Self::default();
3637
for access in raw.iter() {
37-
// Strip workspace root, clean `..` components, and filter in one pass.
38+
// Strip workspace root and clean `..` components in one pass.
3839
// fspy may report paths like `packages/sub-pkg/../shared/dist/output.js`.
3940
let relative_path = access.path.strip_path_prefix(workspace_root, |strip_result| {
4041
let Ok(stripped_path) = strip_result else {
4142
return None;
4243
};
43-
normalize_tracked_workspace_path(stripped_path, resolved_negatives)
44+
normalize_tracked_workspace_path(stripped_path)
4445
});
4546

4647
let Some(relative_path) = relative_path else {
@@ -75,10 +76,7 @@ impl TrackedPathAccesses {
7576
clippy::disallowed_types,
7677
reason = "fspy strip_path_prefix exposes std::path::Path; convert to RelativePathBuf immediately"
7778
)]
78-
fn normalize_tracked_workspace_path(
79-
stripped_path: &std::path::Path,
80-
resolved_negatives: &[wax::Glob<'static>],
81-
) -> Option<RelativePathBuf> {
79+
fn normalize_tracked_workspace_path(stripped_path: &std::path::Path) -> Option<RelativePathBuf> {
8280
// On Windows, paths are possible to be still absolute after stripping the workspace root.
8381
// For example: c:\workspace\subdir\c:\workspace\subdir
8482
// Just ignore those accesses.
@@ -94,12 +92,6 @@ fn normalize_tracked_workspace_path(
9492
return None;
9593
}
9694

97-
if !resolved_negatives.is_empty()
98-
&& resolved_negatives.iter().any(|neg| wax::Program::is_match(neg, relative.as_str()))
99-
{
100-
return None;
101-
}
102-
10395
Some(relative)
10496
}
10597

@@ -115,8 +107,7 @@ mod tests {
115107
clippy::disallowed_types,
116108
reason = "normalize_tracked_workspace_path requires std::path::Path for fspy strip_path_prefix output"
117109
)]
118-
let relative_path =
119-
normalize_tracked_workspace_path(std::path::Path::new(r"foo\C:\bar"), &[]);
110+
let relative_path = normalize_tracked_workspace_path(std::path::Path::new(r"foo\C:\bar"));
120111
assert!(relative_path.is_none());
121112
}
122113
}

crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/snapshots.toml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,18 @@ steps = [
114114
# Should be a cache miss due to output config change
115115
{ argv = ["vt", "run", "output-config-change"], comment = "cache miss: output config changed" },
116116
]
117+
118+
# 6. Input negative globs must not drop matching writes from the output archive.
119+
# Here the user excludes `dist/**` from inferred inputs (so rewriting dist/ won't
120+
# mark inputs as modified), but default auto output should still capture dist
121+
# writes and restore them on a cache hit.
122+
[[e2e]]
123+
name = "input_negative_does_not_drop_output_writes"
124+
steps = [
125+
["vt", "run", "input-neg-dist-auto-output"],
126+
["vtt", "rm", "dist/out.txt"],
127+
# Cache hit should restore dist/out.txt
128+
["vt", "run", "input-neg-dist-auto-output"],
129+
["vtt", "print-file", "dist/out.txt"],
130+
]
131+
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# input_negative_does_not_drop_output_writes
2+
3+
## `vt run input-neg-dist-auto-output`
4+
5+
```
6+
$ vtt write-file dist/out.txt built
7+
8+
$ vtt print done
9+
done
10+
11+
---
12+
vt run: 0/2 cache hit (0%). (Run `vt run --last-details` for full details)
13+
```
14+
15+
## `vtt rm dist/out.txt`
16+
17+
```
18+
```
19+
20+
## `vt run input-neg-dist-auto-output`
21+
22+
```
23+
$ vtt write-file dist/out.txt built ◉ cache hit, replaying
24+
25+
$ vtt print done ◉ cache hit, replaying
26+
done
27+
28+
---
29+
vt run: 2/2 cache hit (100%). (Run `vt run --last-details` for full details)
30+
```
31+
32+
## `vtt print-file dist/out.txt`
33+
34+
```
35+
built
36+
```

crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/vite-task.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@
2323
"command": "vtt write-file dist/out.txt built && vtt print done",
2424
"output": ["REPLACE_ME"],
2525
"cache": true
26+
},
27+
"input-neg-dist-auto-output": {
28+
"command": "vtt write-file dist/out.txt built && vtt print done",
29+
"input": [{ "auto": true }, "!dist/**"],
30+
"cache": true
2631
}
2732
}
2833
}

0 commit comments

Comments
 (0)