feat(client): serve getEnv over runner IPC#446
Conversation
wan9chi
force-pushed
the
infra-get-env-ipc
branch
from
87459b6 to
fc06b30
Compare
wan9chi
force-pushed
the
infra-get-env-ipc
branch
2 times, most recently
from
1174ec4 to
e301d62
Compare
wan9chi
force-pushed
the
infra-get-env-ipc
branch
from
e301d62 to
9d3eeb1
Compare
wan9chi
force-pushed
the
graphite-base/446
branch
from
675e0cb to
1ae40ca
Compare
wan9chi
force-pushed
the
infra-get-env-ipc
branch
2 times, most recently
from
1d8eec4 to
01191f8
Compare
wan9chi
force-pushed
the
infra-get-env-ipc
branch
from
01191f8 to
a20f693
Compare
Motivation: The JS client already exposes getEnv, but before this change it always fell back to no-op behavior. Runner-aware tools need a real round-trip way to ask the runner for an env value before those reads can be used for cache tracking. Scope: Add the GetEnv request/response frame, response reading in the Rust client, server-side resolution from the spawned task env map, and a real NAPI getEnv implementation. This PR intentionally does not add tracked envs to cache fingerprints. Verification: - cargo test -p vite_task_server --test integration - UPDATE_SNAPSHOTS=1 cargo test -p vite_task_bin --test e2e_snapshots fetch_env_reads_declared_env -- --ignored - cargo test -p vite_task_bin --test e2e_snapshots fetch_env_reads_declared_env -- --ignored
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01191f8ed1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // TODO(env-track): A later PR in this stack replaces this child env | ||
| // map with the full planned env context so IPC can serve envs even | ||
| // when they were not declared in `env` or `untrackedEnv`. | ||
| &spawn_execution.spawn_command.all_envs, |
There was a problem hiding this comment.
When a cached task declares an output-affecting variable under untrackedEnv, SpawnCommand::all_envs still contains that value, so this handoff lets getEnv("FOO") return it even though this commit does not record env reads in Reports or add them to the cache fingerprint. A task can then cache output that depends on FOO and incorrectly hit after FOO changes; until tracked reads are implemented, serve only fingerprinted envs or force tracked getEnv reads to affect/disable caching.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn get_env(&mut self, name: &OsStr) -> Option<Arc<OsStr>> { | ||
| self.envs.get(name).cloned() |
There was a problem hiding this comment.
On Windows, env names are case-insensitive, and the planner already matches env config case-insensitively while preserving the parent variable's original casing. If the spawned env contains Path but a tool calls getEnv("PATH") (or declares env: ["PATH"]), this exact FxHashMap lookup returns None even though the child process has that variable; use a case-insensitive lookup under cfg(windows) to match spawned-process semantics.
Useful? React with 👍 / 👎.
wan9chi
force-pushed
the
infra-get-env-ipc
branch
from
a20f693 to
ad9922d
Compare
1 participant
Motivation
The JS client exposes
getEnv, but before this slice it still fell back to no-op behavior. Runner-aware tools need a real request/response path for single env reads before those reads can be used for cache tracking.Scope
Add the
GetEnvprotocol frame, Rust client response handling, server-side resolution from the spawned task env map, and a real NAPIgetEnvimplementation. This PR intentionally does not add tracked env reads to cache fingerprints.Verification
cargo test -p vite_task_server --test integrationUPDATE_SNAPSHOTS=1 cargo test -p vite_task_bin --test e2e_snapshots fetch_env_reads_declared_env -- --ignoredcargo test -p vite_task_bin --test e2e_snapshots fetch_env_reads_declared_env -- --ignored