fix(runner): address PR review on env tracking + client API

wan9chi · claude · wan9chi · commit 7ba7e0a432a6 · 2026-05-14T19:11:42.000+08:00
- Rename the `@voidzero-dev/vite-task-client` exports `fetchEnv`/`fetchEnvs`
  to `getEnv`/`getEnvs`: the calls are synchronous, so the `fetch*` naming
  was misleading (per review).
- `getEnv` now always consults the runner, even when `process.env[name]`
  is already set, so the dependency is still recorded for cache
  invalidation when the value was injected by the shell/prefix env.
- `collect_tracked_env_globs` no longer drops names already covered by the
  user's declared `env`. Lookup-time validation re-expands the glob over
  the whole parent env, so a filtered match-set always diffed as having
  `added` entries and missed the cache deterministically for tasks that
  both declare `env` and call `getEnvs` on an overlapping pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs
@@ -826,10 +826,8 @@ pub async fn execute_spawn(
                         .as_ref()
                         .map(|r| collect_tracked_envs(r, metadata))
                         .unwrap_or_default();
-                    let tracked_env_globs = reports
-                        .as_ref()
-                        .map(|r| collect_tracked_env_globs(r, metadata))
-                        .unwrap_or_default();
+                    let tracked_env_globs =
+                        reports.as_ref().map(collect_tracked_env_globs).unwrap_or_default();
 
                     // Paths already in globbed_inputs are skipped: the overlap check
                     // above guarantees no input modification, so the prerun hash is
@@ -955,33 +953,32 @@ fn collect_tracked_envs(reports: &Reports, metadata: &CacheMetadata) -> BTreeMap
 }
 
 /// Select tool-reported env-glob records to embed in the post-run
-/// fingerprint. Only `tracked: true` records are included, and within each
-/// matched set, names already covered by the user's declared `env` (via the
-/// spawn fingerprint) are skipped — they're redundant and could cause
-/// spurious cache misses if the user later changes their `env` config.
-fn collect_tracked_env_globs(
-    reports: &Reports,
-    metadata: &CacheMetadata,
-) -> BTreeMap<Str, BTreeMap<Str, Str>> {
-    let fingerprinted = &metadata.spawn_fingerprint.env_fingerprints().fingerprinted_envs;
+/// fingerprint. Only `tracked: true` records are included, and the full
+/// match-set is stored as-is.
+///
+/// Unlike [`collect_tracked_envs`], names already covered by the user's
+/// declared `env` are *not* filtered out: lookup-time validation re-expands
+/// the glob over the whole parent env (see [`PostRunFingerprint::validate`]),
+/// so a filtered match-set would always diff as having `added` entries and
+/// miss the cache deterministically. Storing user-declared names here is
+/// harmless — a change to their value already shifts the spawn fingerprint,
+/// invalidating the cache key before the post-run fingerprint is consulted.
+fn collect_tracked_env_globs(reports: &Reports) -> BTreeMap<Str, BTreeMap<Str, Str>> {
     reports
         .env_glob_records
         .iter()
         .filter(|(_, record)| record.tracked)
         .map(|(pattern, record)| {
-            let filtered: BTreeMap<Str, Str> = record
+            let matches: BTreeMap<Str, Str> = record
                 .matches
                 .iter()
                 .filter_map(|(name, value)| {
                     let name_str = name.to_str()?;
-                    if fingerprinted.contains_key(name_str) {
-                        return None;
-                    }
                     let value_str = value.to_str()?;
                     Some((Str::from(name_str), Str::from(value_str)))
                 })
                 .collect();
-            (Str::from(pattern.as_ref()), filtered)
+            (Str::from(pattern.as_ref()), matches)
         })
         .collect()
 }
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_env.mjs b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_env.mjs
@@ -1,10 +1,10 @@
-import { fetchEnv } from '@voidzero-dev/vite-task-client';
+import { getEnv } from '@voidzero-dev/vite-task-client';
 import { writeFileSync, mkdirSync } from 'node:fs';
 
-// fetchEnv populates process.env from the runner and — with tracked: true —
+// getEnv populates process.env from the runner and — with tracked: true —
 // adds the env to the post-run fingerprint, so a change between runs
 // invalidates the cache.
-fetchEnv('PROBE_ENV', { tracked: true });
+getEnv('PROBE_ENV', { tracked: true });
 const value = process.env.PROBE_ENV ?? '(unset)';
 
 mkdirSync('dist', { recursive: true });
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_envs.mjs b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_envs.mjs
@@ -1,12 +1,12 @@
-import { fetchEnvs } from '@voidzero-dev/vite-task-client';
+import { getEnvs } from '@voidzero-dev/vite-task-client';
 import { writeFileSync, mkdirSync } from 'node:fs';
 
-// fetchEnvs asks the runner for every env matching the glob. The glob (plus
+// getEnvs asks the runner for every env matching the glob. The glob (plus
 // its match-set) becomes part of the post-run fingerprint, so adding,
 // removing, or changing any matching env invalidates the cache on the next
 // run. The non-matching UNRELATED envs set by some test steps must not
 // contribute.
-const matches = fetchEnvs('PROBE_*', { tracked: true });
+const matches = getEnvs('PROBE_*', { tracked: true });
 
 mkdirSync('dist', { recursive: true });
 const sorted = Object.entries(matches).sort(([a], [b]) => a.localeCompare(b));
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml
@@ -91,7 +91,7 @@ steps = [
 [[e2e]]
 name = "fetch_envs_tracks_glob_match_set"
 comment = """
-Exercises `fetchEnvs(pattern, { tracked: true })`. The glob `PROBE_*` and
+Exercises `getEnvs(pattern, { tracked: true })`. The glob `PROBE_*` and
 its match-set snapshot enter the post-run fingerprint: later runs diff the
 current match-set against what was stored and miss on add / remove / change,
 but hit when only non-matching envs differ.
@@ -209,7 +209,7 @@ steps = [
 [[e2e]]
 name = "fetch_env_tracked_invalidates_on_change"
 comment = """
-Exercises `fetchEnv(name, { tracked: true })`. The env value becomes part
+Exercises `getEnv(name, { tracked: true })`. The env value becomes part
 of the post-run fingerprint: the same value still hits, a different value
 misses with `tracked env 'PROBE_ENV' changed`.
 """
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_env_tracked_invalidates_on_change.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_env_tracked_invalidates_on_change.md
@@ -1,6 +1,6 @@
 # fetch_env_tracked_invalidates_on_change
 
-Exercises `fetchEnv(name, { tracked: true })`. The env value becomes part
+Exercises `getEnv(name, { tracked: true })`. The env value becomes part
 of the post-run fingerprint: the same value still hits, a different value
 misses with `tracked env 'PROBE_ENV' changed`.
 
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracks_glob_match_set.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_envs_tracks_glob_match_set.md
@@ -1,6 +1,6 @@
 # fetch_envs_tracks_glob_match_set
 
-Exercises `fetchEnvs(pattern, { tracked: true })`. The glob `PROBE_*` and
+Exercises `getEnvs(pattern, { tracked: true })`. The glob `PROBE_*` and
 its match-set snapshot enter the post-run fingerprint: later runs diff the
 current match-set against what was stored and miss on add / remove / change,
 but hit when only non-matching envs differ.
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/snapshots.toml
@@ -46,7 +46,7 @@ steps = [
 name = "vite_prefix_env_change_invalidates_cache"
 comment = """
 `VITE_MODE` is picked up by Vite's patched `loadEnv`, which asks the runner
-for every `VITE_*` env via `fetchEnvs(pattern, { tracked: true })`. Flipping
+for every `VITE_*` env via `getEnvs(pattern, { tracked: true })`. Flipping
 its value between runs must invalidate the cache AND change the build output
 — Vite's `define` plugin substitutes `import.meta.env.VITE_MODE` at build
 time, so dead-code elimination leaves only the branch matching the value.
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/snapshots/vite_prefix_env_change_invalidates_cache.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/snapshots/vite_prefix_env_change_invalidates_cache.md
@@ -1,7 +1,7 @@
 # vite_prefix_env_change_invalidates_cache
 
 `VITE_MODE` is picked up by Vite's patched `loadEnv`, which asks the runner
-for every `VITE_*` env via `fetchEnvs(pattern, { tracked: true })`. Flipping
+for every `VITE_*` env via `getEnvs(pattern, { tracked: true })`. Flipping
 its value between runs must invalidate the cache AND change the build output
 — Vite's `define` plugin substitutes `import.meta.env.VITE_MODE` at build
 time, so dead-code elimination leaves only the branch matching the value.
diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/vite_build_cache/vite-task.json
@@ -3,7 +3,7 @@
     "build": {
       "command": "vite build",
       // No `"env": [...]` — vite's patched `loadEnv` asks the runner for
-      // every `VITE_*` env via `fetchEnvs`, so the glob + match-set are
+      // every `VITE_*` env via `getEnvs`, so the glob + match-set are
       // fingerprinted automatically.
       "cache": true
     }
diff --git a/docs/runner-task-ipc/index.md b/docs/runner-task-ipc/index.md
@@ -30,7 +30,7 @@ Crate / package responsibilities:
 - `vite_task_client` — sync blocking Rust client; `Client::from_envs` no-ops when the IPC env is absent.
 - `vite_task_client_napi` — NAPI cdylib exposing the client's methods to Node.
 - `materialized_artifact` — shared machinery for embedding the cdylib into `vp` and writing it to a temp file on first use (extracted from `fspy`).
-- `@voidzero-dev/vite-task-client` — npm package with JSDoc-typed wrappers (`ignoreInput`, `ignoreOutput`, `disableCache`, `fetchEnv`, `fetchEnvs`). Lazy-loads the addon and silently no-ops when it can't connect, so tools don't break when run outside `vp`.
+- `@voidzero-dev/vite-task-client` — npm package with JSDoc-typed wrappers (`ignoreInput`, `ignoreOutput`, `disableCache`, `getEnv`, `getEnvs`). Lazy-loads the addon and silently no-ops when it can't connect, so tools don't break when run outside `vp`.
 
 Notes:
 
diff --git a/docs/runner-task-ipc/plan.md b/docs/runner-task-ipc/plan.md
@@ -3,6 +3,6 @@
 1. **Protocol** — `vite_task_ipc_shared`. Define message types and serialization. Everything else depends on this. ✅
 2. **Transport** — `vite_task_server` + `vite_task_client`. Build both sides, test them against each other directly in Rust. ✅
 3. **Extract artifact** — Pull `artifact.rs` out of fspy into a shared crate. Prerequisite for dylib embedding. ✅
-4. **JS bridge** — `vite_task_client_napi` (real impl) + `@voidzero-dev/vite-task-client` (JS wrapper with `fetchEnvs` logic). ✅
+4. **JS bridge** — `vite_task_client_napi` (real impl) + `@voidzero-dev/vite-task-client` (JS wrapper with `getEnvs` logic). ✅
 5. **Runner integration** — Wire into `vite_task` spawn: start server per execution, embed/extract dylib, inject the IPC envs via `serve()`'s returned iterator. ✅
 6. **Cache integration** — Runner consumes the reported data (ignored inputs/outputs, requested envs, disable cache) and adjusts caching behavior. ✅
diff --git a/docs/runner-task-ipc/vite-proposal.md b/docs/runner-task-ipc/vite-proposal.md
@@ -60,7 +60,7 @@ This task config duplicates `envPrefix`. The two configs currently have to be ma
 
 **Proposed Changes**:
 
-Add a call to `vite-task-client`'s `fetchEnvs` **before reading the envs matching `envPrefix`**. If Vite is running outside Vite Task, this calls is a no-op. If Vite is running inside Vite Task, it does two things:
+Add a call to `vite-task-client`'s `getEnvs` **before reading the envs matching `envPrefix`**. If Vite is running outside Vite Task, this calls is a no-op. If Vite is running inside Vite Task, it does two things:
 
 - It tells Vite Task which envs are relevant to the build, so they **become part of the cache fingerprint**.
 - It **populates `process.env`** with those envs, so Vite can see them.
@@ -72,7 +72,7 @@ Add a call to `vite-task-client`'s `fetchEnvs` **before reading the envs matchin
    const parsed = Object.fromEntries(...);
    for (const [key, value] of Object.entries(parsed))
      if (prefixes.some(p => key.startsWith(p))) env[key] = value;
-+  for (const prefix of prefixes) fetchEnvs(`${prefix}*`, { tracked: true });
++  for (const prefix of prefixes) getEnvs(`${prefix}*`, { tracked: true });
    for (const key in process.env)
      if (prefixes.some(p => key.startsWith(p))) env[key] = process.env[key];
 ```
@@ -162,8 +162,8 @@ declare module '@voidzero-dev/vite-task-client' {
   export function ignoreInput(path: string): void;
   export function ignoreOutput(path: string): void;
   export function disableCache(): void;
-  export function fetchEnv(name: string, opts?: { tracked?: boolean }): void;
-  export function fetchEnvs(pattern: string, opts?: { tracked?: boolean }): Record<string, string>;
+  export function getEnv(name: string, opts?: { tracked?: boolean }): void;
+  export function getEnvs(pattern: string, opts?: { tracked?: boolean }): Record<string, string>;
   // Same patterns and options as `tinyglobby`'s `glob`. The third
   // argument is the fallback used when called outside Vite Task; passing
   // it in keeps `tinyglobby` out of this package's dependencies.
@@ -175,7 +175,7 @@ declare module '@voidzero-dev/vite-task-client' {
 }
 ```
 
-`fetchEnv` / `fetchEnvs` populate `process.env` for names Vite Task knows about and that aren't already set, so callers read values back via `process.env[...]` as usual.
+`getEnv` / `getEnvs` populate `process.env` for names Vite Task knows about and that aren't already set, so callers read values back via `process.env[...]` as usual.
 
 ### Runtime mechanics
 
diff --git a/packages/vite-task-client/index.js b/packages/vite-task-client/index.js
@@ -45,34 +45,37 @@ export function disableCache() {
 }
 
 /**
- * Populates `process.env[name]` from the runner if it is not already set.
- * The caller reads the value back via `process.env[name]`.
+ * Asks the runner for the env var `name` and, when it isn't already set in
+ * `process.env`, populates it. The caller reads the value back via
+ * `process.env[name]`.
+ *
+ * The runner is always consulted — even when `process.env[name]` is already
+ * set — so that, with `tracked: true`, the dependency is recorded regardless
+ * of whether the value came from the runner or was already in the
+ * environment.
  *
  * @param {string} name
  * @param {{ tracked?: boolean }} [options]
  */
-export function fetchEnv(name, { tracked = true } = {}) {
-  if (process.env[name] !== undefined) return;
+export function getEnv(name, { tracked = true } = {}) {
   const a = load();
   if (!a) return;
   const value = a.getEnv(name, tracked);
-  if (value != null) process.env[name] = value;
+  if (value != null && process.env[name] === undefined) {
+    process.env[name] = value;
+  }
 }
 
 /**
  * Asks the runner for every env whose name matches `pattern` (a glob, e.g.
  * `VITE_*`) and returns the match-set as a plain object. Entries are also
  * written to `process.env` for names that aren't already set.
  *
- * Unlike {@link fetchEnv}, this always round-trips — the client cannot know
- * in advance which names the pattern matches, so can't short-circuit on
- * `process.env`.
- *
  * @param {string} pattern
  * @param {{ tracked?: boolean }} [options]
  * @returns {Record<string, string>}
  */
-export function fetchEnvs(pattern, { tracked = true } = {}) {
+export function getEnvs(pattern, { tracked = true } = {}) {
   const a = load();
   if (!a) return {};
   const entries = a.getEnvs(pattern, tracked);
diff --git a/patches/vite.patch b/patches/vite.patch
@@ -9,7 +9,7 @@ index 5be94a01d8aecf2502e76c05087b207980f2b06d..e7fb05af8e958007277ca137e1b9decb
 +// vp:runner-aware-tools — report inputs/outputs/tracked envs to the runner so
 +// that caching of `vite build` works without manual input/output config.
 +// No-op when `@voidzero-dev/vite-task-client` is not connected to a runner.
-+import { ignoreInput, ignoreOutput, fetchEnvs } from "@voidzero-dev/vite-task-client";
++import { ignoreInput, ignoreOutput, getEnvs } from "@voidzero-dev/vite-task-client";
  import { parseAst, parseAstAsync } from "rolldown/parseAst";
  import { esmExternalRequirePlugin, esmExternalRequirePlugin as esmExternalRequirePlugin$1 } from "rolldown/plugins";
  import { TsconfigCache, Visitor, minify, minifySync, parse, parseSync, transformSync } from "rolldown/utils";
@@ -19,10 +19,10 @@ index 5be94a01d8aecf2502e76c05087b207980f2b06d..e7fb05af8e958007277ca137e1b9decb
  	for (const [key, value] of Object.entries(parsed)) if (prefixes.some((prefix) => key.startsWith(prefix))) env[key] = value;
 +	// vp:runner-aware-tools — ask the runner for every env matching each
 +	// prefix. The match-set becomes part of the post-run fingerprint, so any
-+	// add/remove/change to a matching env invalidates the cache. `fetchEnvs`
++	// add/remove/change to a matching env invalidates the cache. `getEnvs`
 +	// populates `process.env` for names not already set, letting the loop
 +	// below pick them up alongside inline inherited envs.
-+	for (const prefix of prefixes) fetchEnvs(`${prefix}*`, { tracked: true });
++	for (const prefix of prefixes) getEnvs(`${prefix}*`, { tracked: true });
  	for (const key in process.env) if (prefixes.some((prefix) => key.startsWith(prefix))) env[key] = process.env[key];
  	debug$12?.(`using resolved env: %O`, env);
  	return env;
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@`
`3`	`3`	`"build": {`
`4`	`4`	`"command": "vite build",`
`5`	`5`	// No `"env": [...]` — vite's patched `loadEnv` asks the runner for
`6`		- // every `VITE_*` env via `fetchEnvs`, so the glob + match-set are
	`6`	+ // every `VITE_*` env via `getEnvs`, so the glob + match-set are
`7`	`7`	`// fingerprinted automatically.`
`8`	`8`	`"cache": true`
`9`	`9`	`}`