Skip to content

feat: auto output restoration & env detection - #410

Closed
wan9chi wants to merge 10 commits into
mainfrom
feat/runner-tools
Closed

feat: auto output restoration & env detection#410
wan9chi wants to merge 10 commits into
mainfrom
feat/runner-tools

Conversation

@wan9chi

@wan9chiwan9chi commented May 28, 2026

Copy link
Copy Markdown
Member

vp run now supports two cache-correctness features that don't require listing every output/env in the task config:

  1. Auto output restoration. With fspy on, the cache archives the files the task wrote and restores them on a hit — no output: [...] config needed. Tools can opt scratch paths out per-path via ignoreOutput().
  2. Auto env detection. Tools can fetch env values via getEnv / getEnvs and have the runner register the read as a cache-key dependency in the same call — no env: [...] config needed.

Both rely on a new IPC channel between the runner and the tools it spawns. The JS-side surface — the @voidzero-dev/vite-task-client package — shipped in #409 and is now on npm. This PR is the runner side: the IPC protocol, the napi addon the JS wrapper loads at runtime, the executor reshape that hosts a per-task server, the cache integration that consumes the recorded reports, and end-to-end tests.

disableCache() and ignoreInput() round out the tool-side API.

Reviewing

5 commits, one per logical layer — each builds and tests on its own, so commit-by-commit review is recommended:

#CommitWhat it adds
1feat(ipc): protocol + Rust transportvite_task_ipc_shared (wire format), vite_task_server (per-task async server), vite_task_client (sync blocking client). End-to-end integration tests.
2feat(client): napi bindingcrates/vite_task_client_napi — napi addon exposing the Rust client to JS as the RunnerClient returned from load(). (The npm wrapper itself shipped in #409.)
3refactor(execute): reshape for IPC server lifecycleReshape vite_task::session::execute and supporting modules to host a per-task IPC server. The server is still a placeholder here — runner behavior is byte-for-byte identical to today's. Also folds in the blocking output-config refactor.
4feat(cache): integrate runner-aware IPC reportsWire up the real serve(Recorder::new(env_map)), embed the napi addon, inject VP_RUN_NODE_CLIENT_PATH into the child, and merge the resulting Reports into the post-run fingerprint and cache update.
5test(cache): real-vite integration fixtures + playgroundEnd-to-end fixtures exercising the full stack against a real Vite build.

Related

@wan9chiGraphite App

wan9chi commented May 28, 2026

Copy link
Copy Markdown
MemberAuthor

@socket-security

socket-securityBot commented May 28, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

DiffPackageSupply Chain
Security
VulnerabilityQualityMaintenanceLicense
Addedcargo/​napi@​3.9.08210093100100
Addedcargo/​napi-build@​2.3.29810093100100
Addedcargo/​napi-derive@​3.5.69910093100100

View full report

@wan9chi
wan9chiforce-pushed the feat/vite-task-client-js branch from 2fd44c0 to 7a668fdCompareMay 28, 2026 13:55
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 1fed8fa to 93aa7c1CompareMay 28, 2026 13:55
@wan9chi
wan9chiforce-pushed the feat/vite-task-client-js branch from 7a668fd to fcfd107CompareMay 28, 2026 14:13
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch 2 times, most recently from 976fad0 to 9583e9dCompareMay 28, 2026 14:20
@wan9chi
wan9chiforce-pushed the feat/vite-task-client-js branch from fcfd107 to 3061c61CompareMay 28, 2026 14:20
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 9583e9d to 2def2dbCompareMay 28, 2026 14:24
@wan9chi
wan9chiforce-pushed the feat/vite-task-client-js branch from 3061c61 to 2bc64fbCompareMay 28, 2026 14:24
wan9chi added a commit that referenced this pull request May 28, 2026
Lays the groundwork for publishing `@voidzero-dev/vite-task-client` to
npm. JS-side only — `src/index.js` (JSDoc as source of truth),
`src/index.d.ts` regenerated from it, and the metadata required to
publish. No Rust changes; no runner integration.
The package is a thin wrapper with no `dependencies`: under a
`vp run` task it loads the runner-provided napi addon at runtime,
otherwise every method is a graceful no-op.
## Next steps
1. Publish `@voidzero-dev/vite-task-client@0.1.0` to npm.
2. Update the [Vite PR](vitejs/vite#22453) to
depend on the published `@voidzero-dev/vite-task-client`.
3. Merge #410 — the remaining Rust implementation (IPC protocol +
transport, napi binding, executor refactor, cache integration, e2e
tests).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Base automatically changed from feat/vite-task-client-js to mainMay 28, 2026 14:38
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 2def2db to fef1237CompareMay 28, 2026 14:39
@wan9chiwan9chi changed the title feat(ipc): protocol + Rust transportfeat: runner-aware toolsJun 1, 2026
@wan9chi
wan9chi marked this pull request as ready for review June 1, 2026 03:09
@wan9chiwan9chi changed the title feat: runner-aware toolsfeat: auto output restoration for vite buildJun 1, 2026
@wan9chiwan9chi changed the title feat: auto output restoration for vite buildfeat: auto output restoration & env detectionJun 1, 2026

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:fef1237ff3

ℹ️ 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".

Comment on lines 33 to +40
pub inferred_inputs: HashMap<RelativePathBuf, PathFingerprint>,

/// Env vars observed via runner-aware IPC `getEnv` with `tracked: true`.
/// Key is the env name; value is the env value at execution time (or
/// `None` if unset). Validated at cache lookup by comparing against the
/// current parent env.
pub tracked_envs: BTreeMap<Str, Option<Str>>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bump the cache DB version for the new value schema

Adding these fields changes the wincode layout of CacheEntryValue because it embeds PostRunFingerprint, but ExecutionCache::load_from_path still treats existing user_version = 13 databases as current. On an upgrade with a populated v13 cache, a task whose cache key matches an old entry will try to deserialize the old shorter value into this new struct, causing cache lookup to error and the task to fail instead of running or treating it as a miss. Please bump/reset the cache version for this schema change.

Useful? React with 👍 / 👎.

Comment on lines +1001 to +1002
let value = record.value.as_ref().and_then(|v| v.to_str().map(Str::from));
Some((Str::from(name_str), value))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-UTF-8 tracked env values

On Unix, when a native/Rust tool tracks an env var whose value is not valid UTF-8, this conversion stores None, the same representation as an unset variable. Validation uses the same to_str() path, so changing between unset and a non-UTF-8 value, or between two different non-UTF-8 values, will still hit the cache even though the tool observed a different value. Since the protocol and Rust client carry OsStr/NativeStr, the fingerprint should use a byte-faithful representation or explicitly disable caching for such values.

Useful? React with 👍 / 👎.

@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from fef1237 to cf4d4acCompareJune 3, 2026 04:06
@wan9chi
wan9chi changed the base branch from main to graphite-base/410June 5, 2026 08:14
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from cf4d4ac to 80ebb7cCompareJune 5, 2026 08:14
@wan9chi
wan9chi changed the base branch from graphite-base/410 to glob_refactorJune 5, 2026 08:14
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from b6cb7e9 to 4475c88CompareJune 6, 2026 05:23
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 4475c88 to 9f3b3e1CompareJune 6, 2026 15:28
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 9f3b3e1 to 256d384CompareJune 6, 2026 15:33
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 256d384 to 8139380CompareJune 6, 2026 15:34
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 8139380 to 8a5e34eCompareJune 6, 2026 15:50
wan9chi added a commit that referenced this pull request Jun 6, 2026
…negation (#425)
`vite_glob` wraps `wax`, a filesystem path-glob engine — but most
callers use it to match **environment-variable names**, which are flat
strings, not paths. They push `&str` through `is_match(impl
AsRef<Path>)`, which is semantically muddy, and `wax` exposes no option
to disable its path semantics.
This reorganizes the crate into two modules, each with its own matchers
and **separate error type**, and adds **negation** to env-name matching:
- **`vite_glob::env`** — `EnvGlob` (one literal pattern) and
`EnvGlobSet` (a set), backed by `globset` with
`literal_separator(false)` and `case_insensitive(cfg!(windows))`, so
`*`/`?`/`[...]`/`{a,b}` are pure flat-string wildcards with the
platform's env case rules (sensitive on Unix, insensitive on Windows).
`EnvGlobSet` supports **negation**: a `!`-prefixed pattern excludes, and
a name matches when it matches an include and no exclude. Errors:
`EnvGlobError`.
- **`vite_glob::path`** — `PathGlobSet` (the former `GlobPatternSet`,
renamed); still `wax`-backed with unchanged gitignore
(first/last-match-wins) path semantics. Errors: `PathGlobError`.
This mirrors how Turborepo splits env (`regex`) from path (`wax`)
matching, and how Turbo treats `!` in env wildcards. Non-negated env
results are unchanged — globset matches the same `*`/`?`/`[...]`/`{}`
syntax wax did for separator-free names, including `SENSITIVE_PATTERNS`
and the Unix/Windows case split (covered by existing `envs.rs` tests).
### User-facing change
`env: ["VITE_*", "!VITE_SECRET"]` in a task config now fingerprints
every `VITE_*` env **except** `VITE_SECRET`. Previously `!`-prefixed env
patterns were dropped with a warning (no negation). Single-pattern
`EnvGlob` (used by the runner's `getEnvs`) stays literal — `!` has no
special meaning there.
### Migration
Pre-existing callers: `envs.rs` uses `EnvGlobSet` for its multi-pattern
env matching; `vite_workspace` switches to `PathGlobSet` /
`PathGlobError`. The runner-aware env-matching sites consume the
single-pattern `EnvGlob` in the stacked PR #410.
`globset` is a new dependency (BurntSushi / ripgrep); its transitive
deps (`aho-corasick`, `regex`) were already in the tree.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Base automatically changed from glob_refactor to mainJune 6, 2026 15:57
wan9chiand others added 5 commits June 6, 2026 23:57
Add three new crates that together define the runner-tool IPC:
- `vite_task_ipc_shared` — message types + wire format shared by both
ends. Spawned tools tell the runner what they read, wrote, or cared
about; the runner uses that to decide what to fingerprint in the
cache.
- `vite_task_server` — async server hosted in the runner. One server
instance per task execution.
- `vite_task_client` — synchronous blocking client used by spawned
tools. The sync API is deliberate: most tools are JS, called
one-method-at-a-time, and don't want a runtime imposed on them.
The two sides are wired together end-to-end in
`vite_task_server/tests/integration.rs`, so this PR is independently
reviewable as "does the wire format and transport work correctly?"
without depending on runner internals.
Design notes: `docs/runner-task-ipc/`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the JS surface that lets spawned Node tools talk to the runner
over the IPC introduced in the previous PR:
- `crates/vite_task_client_napi` — napi binding that exposes the Rust
`Client` to JS as a `RunnerClient` returned from `load()`.
- `packages/vite-task-client` — JS wrapper with `ignoreInput`,
`ignoreOutput`, `disableCache`, `getEnv`, and `getEnvs`. Types live
in `index.js` as JSDoc; `index.d.ts` is generated via
`pnpm build-vite-task-client-types` and a CI staleness check fails
the build if the committed `.d.ts` drifts from the source.
The package is consumable as a no-op outside the runner: when
`VP_RUN_NODE_CLIENT_PATH` isn't set, `load()` returns `null` and every
exported function becomes a no-op (or returns `undefined`/`{}`). That
means the wrapper is safe to add as a regular dependency from a tool
that wants to opt in to runner-aware behavior without requiring its
users to run under `vp`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reshape `vite_task::session::execute` (and the supporting modules:
`spawn`, `fingerprint`, `cache`, `tracked_accesses`, `event`, and the
summary reporter) to accommodate a per-task IPC server. The server
itself is a placeholder on this branch: instead of actually calling
`vite_task_server::serve(...)`, we construct an empty `Recorder`
whose driver future resolves immediately with no traffic, and bind
a `StopAccepting::noop()`. The downstream plumbing — async-join of
the child with the server, `Reports` flowing into post-run
fingerprinting and into the cache update — is fully in place but
sees only the empty `Reports`, so behaviour is byte-for-byte
identical to today's runner.
The follow-up wires up the real `serve(...)`, embeds the napi
addon, and injects `VP_RUN_NODE_CLIENT_PATH` into the child. That
PR's `serve(Recorder::new(env_map))` is the only call that changes
from "future::ready(Ok(recorder))" to the real bind.
Adds `StopAccepting::noop()` to `vite_task_server` for the same
placeholder use case; it's a tiny helper that's also useful for
tests that need a value of the type without running a server.
Also folds in the unrelated-but-blocking output-config refactor
(`output: Option<UserInputsConfig>` instead of
`Option<Vec<UserOutputEntry>>`) — both sides now route through
`ResolvedGlobConfig::from_user_config`. The 80-odd plan-snapshot
updates are mechanical consequences of that type change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the previous PR's placeholder with the real
`vite_task_server::serve(...)` call: per-task IPC server, napi
addon embedded into the runner binary and materialized to disk on
first use, `VP_RUN_NODE_CLIENT_PATH` injected into the child so the
JS wrapper can `require()` it.
Cache integration: `Reports` collected from the IPC drive
`PostRunFingerprint` and the cache-update path —
- `ignore_input` reads → excluded from input fingerprint
- `ignore_output` writes → excluded from output archive
- `tracked_envs` (single name) + `tracked_env_globs` → folded into
the post-run fingerprint so a value change misses the cache
- `disable_cache` → skips the cache-update path entirely
(`ToolRequested`)
- IPC server bind/runtime failure → `IpcServerError` cache disable
End-to-end coverage via the `ipc_client_test` fixture set: one
fixture per API method, each exercising the real Rust ↔ JS path and
asserting the right cache behaviour. Adds `vtt` (test-only)
helpers `grep_file` and `stat_file` that the fixtures need.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the real-vite e2e coverage: `vite_build_cache` exercises
runner-aware cache behaviour through `vite build` against the
companion vite PR (NODE_ENV-change invalidation, `envPrefix`-driven
tracked-env set, `dist/` write restoration on cache hit), and
`vite_dev_disable_cache` verifies `disableCache()` short-circuits
the dev server's run from the cache.
Wires the rest of the consumer-side bits needed for vite (consumed
via the `github:` spec for vite-task-client) to install cleanly:
- Adds `vite: catalog:` to `packages/tools/package.json` so
`pnpm install` materialises `packages/tools/node_modules/.bin/vite`
for the harness's `link_tools_packages(stage, ["vite"])` symlink
setup.
- Pins root `packageManager` to `pnpm@10.33.4` matching the vite
PR's pnpm, removing a per-install version switch that produced
an outer-vs-inner lockfile-settings mismatch when vite consumed
vite-task as a git dep.
- Drops the root `prepare: husky` so pnpm's git-dep flow doesn't
invoke `pnpm pack`, which had been producing a lockfile
`resolution` entry without the `path:` field — pnpm then
extracted the *entire* vite-task monorepo into
`node_modules/@voidzero-dev/vite-task-client/` instead of the
subpath.
- Pins the vite catalog entry to a commit-specific `pkg.pr.new`
URL so future bumps require updating this line.
- Sets `blockExoticSubdeps: false` so pnpm 11 doesn't reject the
transitive `github:` spec.
Refreshes the playground to use the published vite + the runner-
aware tools.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 8a5e34e to 3f0420fCompareJune 6, 2026 15:57
wan9chiand others added 2 commits June 7, 2026 00:15
…atch
The cache-lookup path hand-rolled a 3-arm match to turn a
`fingerprint::PostRunMismatch` into a `FingerprintMismatch` (identical
variants/fields). Move it into `impl From<PostRunMismatch> for
FingerprintMismatch` so the call site is just `mismatch.into()` and the
mapping lives with the type.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
napi-derive emits `napi::bindgen_prelude::tracing` references when its
`tracing` feature is active. In a workspace where another napi consumer
(e.g. rolldown's binding in Vite+) turns that on via feature unification,
`vite_task_client_napi` failed to compile because its `napi` dependency
lacked the matching `tracing` feature. Enable it so the embedded addon
builds regardless of how napi-derive's features are unified downstream.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
wan9chiand others added 2 commits June 7, 2026 02:13
…ture
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This crate is embedded as a cdylib artifact dependency of `vite_task`.
napi-derive's `type-def` feature is force-enabled by feature unification with
consumers that need it, so it would otherwise write this crate's `#[napi]`
items (RunnerClient/load) into the consumer's shared NAPI_TYPE_DEF_TMP_FOLDER,
which @napi-rs/cli sweeps into the consumer's generated binding as dead exports
(the symbols live in the separately-loaded addon, not the consumer's .node).
build.rs redirects this crate's type-def emission to an isolated sink and prunes
any stale entry a pre-redirect build left in the consumer's (reused) folder. The
public JS surface is the hand-written @voidzero-dev/vite-task-client package, so
these generated defs are never needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@wan9chi
wan9chiforce-pushed the feat/runner-tools branch from 796fa30 to c44d7c3CompareJune 7, 2026 02:18
Runner-aware tools (vite via vite-task-client) report env changes through
getEnv/getEnvs. Previously these rendered as "tracked env '{name}' changed" /
"tracked env glob '{pattern}' changed", which leaks an implementation detail
(how the change was discovered) into user-facing output and diverges from the
manual `env` config wording.
Render them identically to manual env-config changes: the inline cache-miss
reason becomes "envs changed", and the summary detail reuses format_spawn_change
(e.g. "env VITE_X value changed from 'a' to 'b'"). No "tracked" wording.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@wan9chi

wan9chi commented Jun 14, 2026

Copy link
Copy Markdown
MemberAuthor

Closing this PR because it has been replaced by the new Graphite PR stack with smaller, self-contained PRs: https://app.graphite.com/github/pr/voidzero-dev/vite-task/454

@wan9chiwan9chi closed this Jun 14, 2026
@wan9chi
wan9chi deleted the feat/runner-tools branch June 14, 2026 03:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@wan9chi