fix(cache): fail the run when a cache hit's outputs can't be restored

CHANGELOG.md

@@ -4,7 +4,7 @@
 - **Changed** The run summary now says a task that wrote a file it also read was `not cached because it modified its inputs`, and the statistics in `vp run --verbose` and `vp run --last-details` use the singular for a count of one, e.g. `1 task • 1 cache miss` ([#783](https://github.com/voidzero-dev/vite-task/pull/783)).
 - **Fixed** An invalid glob in `--filter` no longer shows its error message twice ([#763](https://github.com/voidzero-dev/vite-task/pull/763)).
 - **Changed** The detailed summary from `vp run --verbose` and `vp run --last-details` now shows each underlying cause of an error on its own line ([#761](https://github.com/voidzero-dev/vite-task/pull/761)).
-- **Added** Remote caching. Configure an endpoint with the workspace's `cache: { remote: { url } }` or `VP_REMOTE_CACHE_URL`, and choose access with `--remote-cache=off|read|read-write` or `VP_REMOTE_CACHE`. The default is `read` with an endpoint and `off` without one. After a local cache miss, `vp run` looks the task up in the remote cache and, on a hit, restores its outputs and caches it locally. The task output and the run summary show which hits came from the remote cache. A failed read is just a cache miss, with the failure as its reason. In `read-write` mode, `vp run` also uploads the results of successful, cacheable tasks after caching them locally. A failed upload doesn't fail the task; the run summary shows a warning instead. Ctrl-C, or a failing task, stops remote cache requests right away, and a task still being looked up doesn't start. Tasks can opt out with `cache: { remote: false }`. Requests use the proxy environment variables or, on macOS and Windows, the system proxy settings ([#727](https://github.com/voidzero-dev/vite-task/pull/727), [#755](https://github.com/voidzero-dev/vite-task/pull/755), [#756](https://github.com/voidzero-dev/vite-task/pull/756), [#757](https://github.com/voidzero-dev/vite-task/pull/757), [#764](https://github.com/voidzero-dev/vite-task/pull/764), [#771](https://github.com/voidzero-dev/vite-task/pull/771), [#772](https://github.com/voidzero-dev/vite-task/pull/772), [#786](https://github.com/voidzero-dev/vite-task/pull/786)).
+- **Added** Remote caching. Configure an endpoint with the workspace's `cache: { remote: { url } }` or `VP_REMOTE_CACHE_URL`, and choose access with `--remote-cache=off|read|read-write` or `VP_REMOTE_CACHE`. The default is `read` with an endpoint and `off` without one. After a local cache miss, `vp run` looks the task up in the remote cache and, on a hit, restores its outputs and caches it locally. The task output and the run summary show which hits came from the remote cache. A failed read is just a cache miss, with the failure as its reason. In `read-write` mode, `vp run` also uploads the results of successful, cacheable tasks after caching them locally. A failed upload doesn't fail the task; the run summary shows a warning instead. Ctrl-C, or a failing task, stops remote cache requests right away, and a task still being looked up doesn't start. Tasks can opt out with `cache: { remote: false }`. Requests use the proxy environment variables or, on macOS and Windows, the system proxy settings ([#727](https://github.com/voidzero-dev/vite-task/pull/727), [#755](https://github.com/voidzero-dev/vite-task/pull/755), [#756](https://github.com/voidzero-dev/vite-task/pull/756), [#757](https://github.com/voidzero-dev/vite-task/pull/757), [#764](https://github.com/voidzero-dev/vite-task/pull/764), [#770](https://github.com/voidzero-dev/vite-task/pull/770), [#771](https://github.com/voidzero-dev/vite-task/pull/771), [#772](https://github.com/voidzero-dev/vite-task/pull/772), [#786](https://github.com/voidzero-dev/vite-task/pull/786)).
 - **Fixed** On Windows, environment variable names used by `vp run` now match regardless of ASCII letter case. Assignments in task commands override earlier assignments and inherited variables spelled differently, and `FORCE_COLOR`, `VP_RUN_CONCURRENCY_LIMIT`, and variables requested through `@voidzero-dev/vite-task-client` are found under any spelling ([#747](https://github.com/voidzero-dev/vite-task/pull/747)).
 - **Changed** A task's cache settings now go inside `cache`, e.g. `cache: { env: ["NODE_ENV"], input: ["src/**"] }`; `cache: true` is the same as `cache: {}`. `env`, `untrackedEnv`, `input`, and `output` are no longer supported at the top level of a task ([#749](https://github.com/voidzero-dev/vite-task/pull/749)).
 - **Fixed** Cached tasks on macOS no longer intermittently fail with exit 2 and `oils I/O error (main): No such process` when a fast command finishes before the shell gets scheduled. The bundled shell that runs task commands is updated to Oils 0.38.0, which fixes this race ([#702](https://github.com/voidzero-dev/vite-task/issues/702), [#703](https://github.com/voidzero-dev/vite-task/pull/703)).

crates/vt/src/session/cache/mod.rs

@@ -127,7 +127,8 @@ pub enum CacheHitSource {
     /// The local cache.
     #[default]
     Local,
-    /// The remote cache. The entry has since been recorded locally.
+    /// The remote cache. The entry is recorded locally once its outputs are
+    /// restored.
     Remote,
 }
 
@@ -315,10 +316,11 @@ impl ExecutionCache {
     /// Returns `Ok(Ok(cache_hit))` on cache hit, `Ok(Err(cache_miss))` on miss.
     ///
     /// After a local miss, the remote cache is queried if the task has one. A
-    /// remote hit is recorded locally, with its output archive downloaded into
-    /// `cache_dir`, and is never uploaded. If the local cache has an entry for
-    /// the task, its miss reason is kept. Otherwise the reason comes from the
-    /// remote cache. Remote requests stop when `cancel_token` is cancelled.
+    /// remote hit has its output archive downloaded into `cache_dir`, is
+    /// recorded locally by [`Self::restore`], and is never uploaded. If the
+    /// local cache has an entry for the task, its miss reason is kept.
+    /// Otherwise the reason comes from the remote cache. Remote requests stop
+    /// when `cancel_token` is cancelled.
     #[tracing::instrument(level = "debug", skip_all)]
     pub async fn try_hit(
         &self,
@@ -408,10 +410,10 @@ impl ExecutionCache {
     }
 
     /// Fetch the entry from the remote cache at `endpoint`. An exact entry
-    /// that passes validation is a hit once its output archive is downloaded
-    /// and the entry is recorded locally. A fallback entry, a failed
-    /// validation, or a failed read is a miss. An error while validating
-    /// counts as a failed read, so the remote entry never fails the task.
+    /// that passes validation is a hit once its output archive is downloaded.
+    /// A fallback entry, a failed validation, or a failed read is a miss. An
+    /// error while validating counts as a failed read, so the remote entry
+    /// never fails the task.
     #[expect(clippy::too_many_arguments, reason = "forwarded from `try_hit`")]
     async fn try_hit_remote(
         &self,
@@ -449,10 +451,7 @@ impl ExecutionCache {
             }
             None => None,
         };
-        let cache_value = CacheEntryValue { output_archive, ..cache_value };
-        self.record(cache_key, &cache_metadata.execution_cache_key, &cache_value, cache_dir)
-            .await?;
-        Ok(Ok(cache_value))
+        Ok(Ok(CacheEntryValue { output_archive, ..cache_value }))
     }
 
     /// Record an entry locally.
@@ -519,6 +518,44 @@ impl ExecutionCache {
         }
         Ok(upload)
     }
+
+    /// Restore the output files of `hit` into `workspace_root`.
+    ///
+    /// A remote hit is recorded locally once its outputs are restored. If
+    /// they can't be, its downloaded archive is removed instead, so the next
+    /// run fetches it again. Returns an error if the outputs can't be
+    /// restored.
+    pub async fn restore(
+        &self,
+        cache_metadata: &CacheMetadata,
+        hit: &CacheHit,
+        workspace_root: &AbsolutePath,
+        cache_dir: &AbsolutePath,
+    ) -> anyhow::Result<()> {
+        if let Some(archive_name) = &hit.value.output_archive {
+            let archive_path = cache_dir.join(archive_name.as_str());
+            if let Err(err) = archive::extract_output_archive(workspace_root, &archive_path) {
+                if hit.source == CacheHitSource::Remote {
+                    // Best-effort: the file may already be missing.
+                    let _ = std::fs::remove_file(archive_path.as_path());
+                }
+                return Err(err.context("failed to extract the output archive"));
+            }
+        }
+
+        if hit.source == CacheHitSource::Remote {
+            let cache_key = CacheEntryKey::from_metadata(cache_metadata);
+            if let Err(err) = self
+                .record(&cache_key, &cache_metadata.execution_cache_key, &hit.value, cache_dir)
+                .await
+            {
+                // The outputs are restored, so the task still succeeds. The
+                // next run fetches the entry from the remote cache again.
+                tracing::warn!(?err, "failed to record a remote cache hit locally");
+            }
+        }
+        Ok(())
+    }
 }
 
 // Basic database operations

crates/vt/src/session/event.rs

@@ -2,6 +2,7 @@ use std::{process::ExitStatus, time::Duration};
 
 use vt_path::RelativePathBuf;
 use vt_server::Error as IpcServerError;
+use vt_str::Str;
 
 use super::cache::{CacheHitSource, CacheMiss, remote::UploadError};
 
@@ -10,6 +11,9 @@ use super::cache::{CacheHitSource, CacheMiss, remote::UploadError};
 pub enum CacheErrorKind {
     /// Cache lookup (`try_hit`) failed.
     Lookup,
+    /// Restoring the output files of a remote cache hit failed. A local hit
+    /// fails with [`ExecutionError::LocalCacheRestore`] instead.
+    Restore,
     /// Writing the cache entry failed after successful execution.
     Update,
 }
@@ -18,6 +22,7 @@ impl std::fmt::Display for CacheErrorKind {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::Lookup => f.write_str("lookup"),
+            Self::Restore => f.write_str("restore"),
             Self::Update => f.write_str("update"),
         }
     }
@@ -37,6 +42,16 @@ pub enum ExecutionError {
         source: anyhow::Error,
     },
 
+    /// Restoring the output files of a local cache hit failed. The entry
+    /// stays in the cache, so later runs fail the same way until the cache is
+    /// cleared.
+    #[error("Cache restore failed. Run `{program_name} cache clean` to clear the cache")]
+    LocalCacheRestore {
+        program_name: Str,
+        #[source]
+        source: anyhow::Error,
+    },
+
     /// The OS failed to spawn the child process (e.g., command not found).
     #[error("Failed to spawn process")]
     Spawn(#[source] anyhow::Error),

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

@@ -31,7 +31,7 @@ use self::{
     spawn::{ChildHandle, ChildOutcome, SpawnStdio, spawn},
 };
 use super::{
-    cache::{CacheEntryValue, CacheHit, CacheMiss, ExecutionCache, archive},
+    cache::{CacheHit, CacheHitSource, CacheMiss, ExecutionCache},
     event::{
         CacheDisabledReason, CacheErrorKind, CacheNotUpdatedReason, CacheStatus, CacheUpdateStatus,
         ExecutionError,
@@ -410,16 +410,21 @@ async fn run(
     //    runs exactly once on every arm) and either replay the hit — no need
     //    to execute the command — or carry the globbed inputs into the run.
     let (stdio_config, globbed_inputs) = match lookup {
-        CacheLookup::Hit(CacheHit { value: cached, source }) => {
-            let mut stdio_config =
-                reporter.start(CacheStatus::Hit { replayed_duration: cached.duration, source });
+        CacheLookup::Hit { hit, metadata } => {
+            let mut stdio_config = reporter.start(CacheStatus::Hit {
+                replayed_duration: hit.value.duration,
+                source: hit.source,
+            });
             return Ok(replay_cache_hit(
                 &mut stdio_config,
-                &cached,
+                &hit,
+                cache,
+                metadata,
                 workspace_root,
                 cache_dir,
                 program_name,
-            ));
+            )
+            .await);
         }
         CacheLookup::Miss { miss, globbed_inputs } => {
             (reporter.start(CacheStatus::Miss(miss)), globbed_inputs)
@@ -525,9 +530,10 @@ async fn run(
 /// outcome provides: a hit owns the cached entry to replay, a miss keeps the
 /// reason plus the globbed inputs (reused by the cache-update phase after the
 /// run), and disabled has neither.
-enum CacheLookup {
-    /// Cache hit — the cached entry to replay, and where it came from.
-    Hit(CacheHit),
+enum CacheLookup<'a> {
+    /// Cache hit — the cached entry to replay, where it came from, and the
+    /// metadata that looked it up.
+    Hit { hit: CacheHit, metadata: &'a CacheMetadata },
     /// Cache miss — the detailed reason (`NotFound` or `FingerprintMismatch`).
     Miss { miss: CacheMiss, globbed_inputs: BTreeMap<RelativePathBuf, u64> },
     /// Caching is disabled for this task (no cache metadata).
@@ -537,13 +543,13 @@ enum CacheLookup {
 /// Phase 1: compute the globbed inputs and try to hit the cache. A remote hit
 /// downloads its output archive into `cache_dir`. Remote requests stop when
 /// `cancel_token` is cancelled.
-async fn lookup_cache(
-    cache_metadata: Option<&CacheMetadata>,
+async fn lookup_cache<'a>(
+    cache_metadata: Option<&'a CacheMetadata>,
     cache: &ExecutionCache,
     workspace_root: &Arc<AbsolutePath>,
     cache_dir: &AbsolutePath,
     cancel_token: &CancellationToken,
-) -> Result<CacheLookup, Report> {
+) -> Result<CacheLookup<'a>, Report> {
     let Some(cache_metadata) = cache_metadata else {
         return Ok(CacheLookup::Disabled);
     };
@@ -563,7 +569,7 @@ async fn lookup_cache(
         .try_hit(cache_metadata, &globbed_inputs, workspace_root, cache_dir, cancel_token)
         .await
     {
-        Ok(Ok(cached)) => Ok(CacheLookup::Hit(cached)),
+        Ok(Ok(hit)) => Ok(CacheLookup::Hit { hit, metadata: cache_metadata }),
         Ok(Err(miss)) => Ok(CacheLookup::Miss { miss, globbed_inputs }),
         Err(err) => {
             Err(Report::failed(ExecutionError::Cache { kind: CacheErrorKind::Lookup, source: err }))
@@ -572,15 +578,17 @@ async fn lookup_cache(
 }
 
 /// Phase 3 (cache hit): replay the captured stdout/stderr and restore the
-/// output archive.
-fn replay_cache_hit(
+/// output files.
+async fn replay_cache_hit(
     stdio_config: &mut StdioConfig,
-    cached: &CacheEntryValue,
+    hit: &CacheHit,
+    cache: &ExecutionCache,
+    cache_metadata: &CacheMetadata,
     workspace_root: &Arc<AbsolutePath>,
     cache_dir: &AbsolutePath,
     program_name: &str,
 ) -> Report {
-    for output in cached.std_outputs.iter() {
+    for output in hit.value.std_outputs.iter() {
         let writer: &mut dyn std::io::Write = match output.kind {
             pipe::OutputKind::StdOut => &mut stdio_config.writers.stdout_writer,
             pipe::OutputKind::StdErr => &mut stdio_config.writers.stderr_writer,
@@ -589,24 +597,23 @@ fn replay_cache_hit(
         let _ = writer.flush();
     }
 
-    // Restore output files from the cached archive. Failure here means the
-    // archive file is missing, truncated, or otherwise unreadable — the
-    // task can't proceed because the cache promised the outputs would be
-    // restored. Surface a recovery instruction rather than just the raw
-    // I/O error so users know to clear the cache.
-    if let Some(ref archive_name) = cached.output_archive {
-        let archive_path = cache_dir.join(archive_name.as_str());
-        if let Err(err) = archive::extract_output_archive(workspace_root, &archive_path) {
-            let err = err.context(vt_str::format!(
-                "failed to restore cached outputs from {}; the archive may have been deleted \
-                 or corrupted. Run `{program_name} cache clean` to clear the cache.",
-                archive_path.as_path().display()
-            ));
-            return Report::Failed {
-                cache_update: CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheHit),
-                error: ExecutionError::Cache { kind: CacheErrorKind::Lookup, source: err },
-            };
-        }
+    // Failure here means the archive is missing or unreadable, or its files
+    // can't be written. The task fails because the cache promised the
+    // outputs would be restored.
+    if let Err(err) = cache.restore(cache_metadata, hit, workspace_root, cache_dir).await {
+        let error = match hit.source {
+            CacheHitSource::Local => {
+                ExecutionError::LocalCacheRestore { program_name: program_name.into(), source: err }
+            }
+            // Not recorded locally, so there's no entry to clear.
+            CacheHitSource::Remote => {
+                ExecutionError::Cache { kind: CacheErrorKind::Restore, source: err }
+            }
+        };
+        return Report::Failed {
+            cache_update: CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::CacheHit),
+            error,
+        };
     }
 
     Report::CacheHit