feat: skip caching tasks that modify their inputs (#248)

Author: branchseer

crates/vite_task/src/session/event.rs

@@ -1,5 +1,7 @@
 use std::{process::ExitStatus, time::Duration};
 
+use vite_path::RelativePathBuf;
+
 use super::cache::CacheMiss;
 
 /// The cache operation that failed.
@@ -57,6 +59,12 @@ pub enum CacheNotUpdatedReason {
     CacheDisabled,
     /// Execution exited with non-zero status
     NonZeroExitStatus,
+    /// Task modified files it read during execution (read-write overlap detected by fspy).
+    /// Caching such tasks is unsound because the prerun input hashes become stale.
+    InputModified {
+        /// First path that was both read and written during execution.
+        path: RelativePathBuf,
+    },
 }
 
 #[derive(Debug)]
@@ -66,13 +74,7 @@ pub enum CacheUpdateStatus {
     /// Cache was not updated (with reason).
     /// The reason is part of the `LeafExecutionReporter` trait contract — reporters
     /// can use it for detailed logging, even if current implementations don't.
-    NotUpdated(
-        #[expect(
-            dead_code,
-            reason = "part of LeafExecutionReporter trait contract; reporters may use for detailed logging"
-        )]
-        CacheNotUpdatedReason,
-    ),
+    NotUpdated(CacheNotUpdatedReason),
 }
 
 #[derive(Debug)]

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

@@ -55,19 +55,24 @@ impl PostRunFingerprint {
     /// Creates a new fingerprint from path accesses after task execution.
     ///
     /// Negative glob filtering is done upstream in `spawn_with_tracking`.
-    /// Paths may contain `..` components from fspy, so this method cleans them
-    /// before fingerprinting.
+    /// Paths already present in `globbed_inputs` are skipped — they are
+    /// already tracked by the prerun glob fingerprint, and the read-write
+    /// overlap check in `execute_spawn` guarantees the task did not modify
+    /// them, so the prerun hash is still correct.
     ///
     /// # Arguments
     /// * `inferred_path_reads` - Map of paths that were read during execution (from fspy)
     /// * `base_dir` - Workspace root for resolving relative paths
+    /// * `globbed_inputs` - Prerun glob fingerprint; paths here are skipped
     #[tracing::instrument(level = "debug", skip_all, name = "create_post_run_fingerprint")]
     pub fn create(
         inferred_path_reads: &HashMap<RelativePathBuf, PathRead>,
         base_dir: &AbsolutePath,
+        globbed_inputs: &BTreeMap<RelativePathBuf, u64>,
     ) -> anyhow::Result<Self> {
         let inferred_inputs = inferred_path_reads
             .par_iter()
+            .filter(|(path, _)| !globbed_inputs.contains_key(*path))
             .map(|(relative_path, path_read)| {
                 let full_path = Arc::<AbsolutePath>::from(base_dir.join(relative_path));
                 let fingerprint = fingerprint_path(&full_path, *path_read)?;

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

@@ -397,34 +397,55 @@ pub async fn execute_spawn(
         cache_metadata_and_inputs
     {
         if result.exit_status.success() {
-            // path_reads is empty when inference is disabled (path_accesses is None)
-            let empty_path_reads = HashMap::default();
-            let path_reads = path_accesses.as_ref().map_or(&empty_path_reads, |pa| &pa.path_reads);
-
-            // Execution succeeded — attempt to create fingerprint and update cache
-            match PostRunFingerprint::create(path_reads, cache_base_path) {
-                Ok(post_run_fingerprint) => {
-                    let new_cache_value = CacheEntryValue {
-                        post_run_fingerprint,
-                        std_outputs: std_outputs.unwrap_or_default().into(),
-                        duration: result.duration,
-                        globbed_inputs,
-                    };
-                    match cache.update(cache_metadata, new_cache_value).await {
-                        Ok(()) => (CacheUpdateStatus::Updated, None),
-                        Err(err) => (
-                            CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheDisabled),
-                            Some(ExecutionError::Cache {
-                                kind: CacheErrorKind::Update,
-                                source: err,
-                            }),
-                        ),
+            // Check for read-write overlap: if the task wrote to any file it also
+            // read, the inputs were modified during execution — don't cache.
+            // Note: this only checks fspy-inferred reads, not globbed_inputs keys.
+            // A task that writes to a glob-matched file without reading it causes
+            // perpetual cache misses (glob detects the hash change) but not a
+            // correctness bug, so we don't handle that case here.
+            if let Some(path) = path_accesses
+                .as_ref()
+                .and_then(|pa| pa.path_reads.keys().find(|p| pa.path_writes.contains(*p)))
+            {
+                (
+                    CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::InputModified {
+                        path: path.clone(),
+                    }),
+                    None,
+                )
+            } else {
+                // path_reads is empty when inference is disabled (path_accesses is None)
+                let empty_path_reads = HashMap::default();
+                let path_reads =
+                    path_accesses.as_ref().map_or(&empty_path_reads, |pa| &pa.path_reads);
+
+                // Execution succeeded — attempt to create fingerprint and update cache.
+                // Paths already in globbed_inputs are skipped: Rule 1 (above) guarantees
+                // no input modification, so the prerun hash is the correct post-exec hash.
+                match PostRunFingerprint::create(path_reads, cache_base_path, &globbed_inputs) {
+                    Ok(post_run_fingerprint) => {
+                        let new_cache_value = CacheEntryValue {
+                            post_run_fingerprint,
+                            std_outputs: std_outputs.unwrap_or_default().into(),
+                            duration: result.duration,
+                            globbed_inputs,
+                        };
+                        match cache.update(cache_metadata, new_cache_value).await {
+                            Ok(()) => (CacheUpdateStatus::Updated, None),
+                            Err(err) => (
+                                CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheDisabled),
+                                Some(ExecutionError::Cache {
+                                    kind: CacheErrorKind::Update,
+                                    source: err,
+                                }),
+                            ),
+                        }
                     }
+                    Err(err) => (
+                        CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheDisabled),
+                        Some(ExecutionError::PostRunFingerprint(err)),
+                    ),
                 }
-                Err(err) => (
-                    CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheDisabled),
-                    Some(ExecutionError::PostRunFingerprint(err)),
-                ),
             }
         } else {
             // Execution failed with non-zero exit status — don't update cache

crates/vite_task/src/session/reporter/labeled.rs

@@ -251,7 +251,7 @@ impl LeafExecutionReporter for LabeledLeafReporter {
     async fn finish(
         self: Box<Self>,
         status: Option<StdExitStatus>,
-        _cache_update_status: CacheUpdateStatus,
+        cache_update_status: CacheUpdateStatus,
         error: Option<ExecutionError>,
     ) {
         // Convert error before consuming it (need the original for display formatting).
@@ -276,7 +276,12 @@ impl LeafExecutionReporter for LabeledLeafReporter {
                 task_name: display.task_display.task_name.clone(),
                 command: display.command.clone(),
                 cwd: cwd_relative,
-                result: TaskResult::from_execution(&cache_status, status, saved_error.as_ref()),
+                result: TaskResult::from_execution(
+                    &cache_status,
+                    status,
+                    saved_error.as_ref(),
+                    &cache_update_status,
+                ),
             };
 
             shared.borrow_mut().tasks.push(task_summary);