Skip to content

Commit 700c46a

Browse files
authored
fix(plan): serve runner getEnv from unfiltered env context (#447)
## Motivation Runner-served `getEnv` values need to match the command context that planning and execution use, including command-prefix envs and enclosing nested `vp run` prefix envs. Serving only the cache-filtered child env hides undeclared prefix values from runner-aware tools. ## Scope Build on the `spawn_envs` naming cleanup by storing the unfiltered command env context on `CacheMetadata` for cached executions, then serve runner `getEnv` from that context. `SpawnCommand::spawn_envs` remains focused on the actual child process environment. This PR intentionally does not add cache fingerprinting for runner-served env reads. That is handled by the next PR in the stack. The e2e fixture now uses one generic `fetch_env.mjs <NAME> [...]` helper and compares runner-served values with `process.env` for undeclared envs, command-prefix envs, and nested prefix envs. ## Verification - `cargo test -p vite_task_bin --test e2e_snapshots fetch_env -- --ignored`
1 parent 42cd212 commit 700c46a

10 files changed

Lines changed: 117 additions & 43 deletions

File tree

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use std::{
1818
};
1919

2020
use futures_util::future::LocalBoxFuture;
21-
use rustc_hash::FxHashMap;
2221
use tokio_util::sync::CancellationToken;
2322
use vite_path::{AbsolutePath, RelativePathBuf};
2423
use vite_task_ipc_shared::NODE_CLIENT_PATH_ENV_NAME;
@@ -157,7 +156,6 @@ impl<'a> ExecutionMode<'a> {
157156
cache_metadata: Option<&'a CacheMetadata>,
158157
stdio_config: StdioConfig,
159158
globbed_inputs: BTreeMap<RelativePathBuf, u64>,
160-
envs: &BTreeMap<Arc<OsStr>, Arc<OsStr>>,
161159
) -> Result<Self, ExecutionError> {
162160
let Some(metadata) = cache_metadata else {
163161
return Ok(Self::Uncached {
@@ -184,10 +182,9 @@ impl<'a> ExecutionMode<'a> {
184182
// Bind runner IPC for every cached task. The merged cache-control API
185183
// (`disableCache`) must work even when a task uses explicit inputs and
186184
// therefore does not need fspy auto-input inference.
187-
let server_envs: FxHashMap<Arc<OsStr>, Arc<OsStr>> =
188-
envs.iter().map(|(name, value)| (Arc::clone(name), Arc::clone(value))).collect();
189185
let (ipc_envs, ServerHandle { driver, stop_accepting }) =
190-
serve(Recorder::new(Arc::new(server_envs))).map_err(ExecutionError::IpcServerBind)?;
186+
serve(Recorder::new(Arc::clone(&metadata.unfiltered_envs)))
187+
.map_err(ExecutionError::IpcServerBind)?;
191188
let tracking =
192189
Tracking { fspy, ipc_envs: ipc_envs.collect(), ipc_server_fut: driver, stop_accepting };
193190

@@ -409,16 +406,8 @@ async fn run(
409406
};
410407

411408
// 4. Fold the cache/fspy/stdio decisions into the typed mode.
412-
let mut mode = ExecutionMode::build(
413-
cache_metadata,
414-
stdio_config,
415-
globbed_inputs,
416-
// TODO(env-track): A later PR in this stack replaces this child env
417-
// map with the full planned env context so IPC can serve envs even
418-
// when they were not declared in `env` or `untrackedEnv`.
419-
&spawn_execution.spawn_command.spawn_envs,
420-
)
421-
.map_err(Report::failed)?;
409+
let mut mode = ExecutionMode::build(cache_metadata, stdio_config, globbed_inputs)
410+
.map_err(Report::failed)?;
422411

423412
// Measure end-to-end duration here — spawn() doesn't track time.
424413
let start = Instant::now();
Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import { getEnv } from '@voidzero-dev/vite-task-client';
22

3-
const value = getEnv('PROBE_ENV') ?? '(unset)';
3+
const names = process.argv.slice(2);
44

5-
console.log('PROBE_ENV=' + value);
5+
if (names.length === 0) {
6+
throw new Error('usage: fetch_env.mjs <NAME> [...]');
7+
}
8+
9+
const served = names.map((name) => `${name}=${getEnv(name) ?? '(unset)'}`);
10+
const own = names.map((name) => `${name}=${process.env[name] ?? '(unset)'}`);
11+
12+
console.log(`served ${served.join(' ')}`);
13+
console.log(`process.env ${own.join(' ')}`);

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,48 @@ steps = [
4848
]
4949

5050
[[e2e]]
51-
name = "fetch_env_reads_declared_env"
51+
name = "fetch_env_reads_undeclared_env"
5252
comment = """
53-
Exercises `getEnv(name)`: the tool asks the runner for a declared env var and prints the served value. This verifies the round-trip IPC behavior before any runner-reported env is added to the cache fingerprint.
53+
Exercises `getEnv(name)`: the tool asks the runner for an env var that is not declared in the task's `env` list. This verifies runner-served envs resolve from the unfiltered spawn env context while the child process env remains cache-filtered.
5454
"""
5555
ignore = true
5656
steps = [
5757
{ argv = [
5858
"vt",
5959
"run",
60-
"fetch-env-declared",
60+
"fetch-env",
6161
], envs = [
6262
[
6363
"PROBE_ENV",
6464
"served",
6565
],
66-
], comment = "runner serves PROBE_ENV from the spawned task env map" },
66+
], comment = "runner serves undeclared PROBE_ENV from the unfiltered env context" },
67+
]
68+
69+
[[e2e]]
70+
name = "fetch_env_sees_command_prefix_env"
71+
comment = """
72+
A command-prefixed env (`PREFIXED_ENV=from-command node ...`) is part of the spawn's full env context. `getEnv('PREFIXED_ENV')` and `process.env.PREFIXED_ENV` must both see the command prefix value.
73+
"""
74+
ignore = true
75+
steps = [
76+
{ argv = [
77+
"vt",
78+
"run",
79+
"fetch-prefixed-env",
80+
], comment = "tool asks the runner for an env the command prefix sets" },
81+
]
82+
83+
[[e2e]]
84+
name = "fetch_env_sees_intermediate_prefix_envs"
85+
comment = """
86+
Prefix envs accumulate through nested runs: `outer-prefixed` is `PREFIXED_A=a vt run inner-prefixed`, and `inner-prefixed` is `PREFIXED_B=b node ...`. The inner tool's `getEnv` must see both through the runner's env context, while `process.env` only sees the direct child prefix.
87+
"""
88+
ignore = true
89+
steps = [
90+
{ argv = [
91+
"vt",
92+
"run",
93+
"outer-prefixed",
94+
], comment = "outer prefix env reaches the inner tool via the runner" },
6795
]

crates/vite_task_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/fetch_env_reads_declared_env.md

Lines changed: 0 additions & 12 deletions
This file was deleted.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# fetch_env_reads_undeclared_env
2+
3+
Exercises `getEnv(name)`: the tool asks the runner for an env var that is not declared in the task's `env` list. This verifies runner-served envs resolve from the unfiltered spawn env context while the child process env remains cache-filtered.
4+
5+
## `PROBE_ENV=served vt run fetch-env`
6+
7+
runner serves undeclared PROBE_ENV from the unfiltered env context
8+
9+
```
10+
$ node scripts/fetch_env.mjs PROBE_ENV
11+
served PROBE_ENV=served
12+
process.env PROBE_ENV=(unset)
13+
```
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# fetch_env_sees_command_prefix_env
2+
3+
A command-prefixed env (`PREFIXED_ENV=from-command node ...`) is part of the spawn's full env context. `getEnv('PREFIXED_ENV')` and `process.env.PREFIXED_ENV` must both see the command prefix value.
4+
5+
## `vt run fetch-prefixed-env`
6+
7+
tool asks the runner for an env the command prefix sets
8+
9+
```
10+
$ PREFIXED_ENV=from-command node scripts/fetch_env.mjs PREFIXED_ENV
11+
served PREFIXED_ENV=from-command
12+
process.env PREFIXED_ENV=from-command
13+
```
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# fetch_env_sees_intermediate_prefix_envs
2+
3+
Prefix envs accumulate through nested runs: `outer-prefixed` is `PREFIXED_A=a vt run inner-prefixed`, and `inner-prefixed` is `PREFIXED_B=b node ...`. The inner tool's `getEnv` must see both through the runner's env context, while `process.env` only sees the direct child prefix.
4+
5+
## `vt run outer-prefixed`
6+
7+
outer prefix env reaches the inner tool via the runner
8+
9+
```
10+
$ PREFIXED_B=b node scripts/fetch_env.mjs PREFIXED_A PREFIXED_B
11+
served PREFIXED_A=a PREFIXED_B=b
12+
process.env PREFIXED_A=(unset) PREFIXED_B=b
13+
```

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,20 @@
99
"input": [],
1010
"cache": true
1111
},
12-
"fetch-env-declared": {
13-
"command": "node scripts/fetch_env.mjs",
14-
// TODO(env-track): Remove this explicit env declaration down the stack and update the e2e name once runner-tracked env reads cover it.
15-
"env": ["PROBE_ENV"],
12+
"fetch-env": {
13+
"command": "node scripts/fetch_env.mjs PROBE_ENV",
14+
"cache": true
15+
},
16+
"fetch-prefixed-env": {
17+
"command": "PREFIXED_ENV=from-command node scripts/fetch_env.mjs PREFIXED_ENV",
18+
"cache": true
19+
},
20+
"outer-prefixed": {
21+
"command": "PREFIXED_A=a vt run inner-prefixed",
22+
"cache": true
23+
},
24+
"inner-prefixed": {
25+
"command": "PREFIXED_B=b node scripts/fetch_env.mjs PREFIXED_A PREFIXED_B",
1626
"cache": true
1727
}
1828
}

crates/vite_task_plan/src/cache_metadata.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
use std::sync::Arc;
1+
use std::{ffi::OsStr, sync::Arc};
22

3+
use rustc_hash::FxHashMap;
34
use serde::{Deserialize, Serialize};
45
use vite_path::RelativePathBuf;
56
use vite_str::{self, Str};
@@ -52,6 +53,16 @@ pub struct CacheMetadata {
5253
/// Resolved output configuration for cache restoration.
5354
/// Used at execution time to determine what output files to archive.
5455
pub output_config: ResolvedGlobConfig,
56+
57+
/// The unfiltered env context for runner-aware APIs. This is the planning
58+
/// context's envs before spawn-env filtering, including command prefix envs
59+
/// from this command and enclosing nested `vp run` expansions.
60+
///
61+
/// It is not passed to the spawned process; that remains
62+
/// `SpawnCommand::spawn_envs`. It is skipped in serialized plans because it
63+
/// mirrors the ambient environment and would make snapshots noisy.
64+
#[serde(skip)]
65+
pub unfiltered_envs: Arc<FxHashMap<Arc<OsStr>, Arc<OsStr>>>,
5566
}
5667

5768
/// Fingerprint for spawn execution that affects caching.

crates/vite_task_plan/src/plan.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,11 @@ async fn plan_task_as_execution_node(
143143

144144
// This command's env context: extend the planning context with
145145
// the command's prefix envs (`FOO=1 tool ...`). Everything that
146-
// interprets the command reads this one context program
147-
// lookup, plan-request callbacks, and nested-run planning. The
148-
// per-and-item duplicate above scopes the extension to this
149-
// command, matching shell semantics (`FOO=1 a && b` does not
150-
// set `FOO` for `b`).
146+
// interprets the command reads this one context: program
147+
// lookup, plan-request callbacks, nested-run planning, and
148+
// cached execution metadata. The per-and-item duplicate
149+
// above scopes the extension to this command, matching shell
150+
// semantics (`FOO=1 a && b` does not set `FOO` for `b`).
151151
context.add_envs(and_item.envs.iter());
152152

153153
let mut args = and_item.args;
@@ -673,6 +673,7 @@ fn plan_spawn_execution(
673673
execution_cache_key,
674674
input_config: cache_config.input_config.clone(),
675675
output_config: cache_config.output_config.clone(),
676+
unfiltered_envs: Arc::clone(envs),
676677
});
677678
}
678679
}

0 commit comments

Comments
 (0)