feat: runner-aware tools#346
Draft
branchseer wants to merge 12 commits into
Draft
Conversation
4ba6a19 to
64f1651
Compare
7 tasks
64f1651 to
f6605e3
Compare
0008bd7 to
994624a
Compare
cpojer
reviewed
Apr 20, 2026
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
e18c21a to
09a310f
Compare
589a626 to
9df3054
Compare
09a310f to
87d8c32
Compare
9df3054 to
3a8b605
Compare
4edcd12 to
cfa4282
Compare
3a8b605 to
6198f6b
Compare
24f5889 to
ffca388
Compare
Adds `{ auto: true }` support to the `output` field, plus the implicit
default: when `output` is omitted, automatically tracks files the task
writes (via fspy) and archives them. Explicit globs and `auto` can be
mixed in the same array.
Also includes:
- `read_write_overlap` check: if a task writes to a file it also read
(auto-inferred), the cache update is skipped (`InputModified`).
Prerun input hashes would otherwise be stale.
- Input negatives apply to reads only, not writes — keeps `input: ["!dist/**"]`
from accidentally dropping writes to `dist/**` during archiving.
- Input-auto gating: when `input_config.includes_auto` is false, fspy
reads do not contribute to the post-run fingerprint, even when fspy
is enabled solely for output tracking.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Squashed rebase of 51 commits from runner-aware-tools branch onto feat/output-restoration. Original commit history preserved on the ras-backup ref for reference. Key features bundled: - vite_task_ipc_shared: shared protocol (Request/GetEnvResponse, NativeStr) - vite_task_server: per-task IPC server (Handler trait + Recorder) - vite_task_client: sync Rust client - vite_task_client_napi + @voidzero-dev/vite-task-client: node addon + JS wrapper - vite_task: wire IPC server into spawn; inject VP_IPC + VP_RUN_NODE_CLIENT_PATH; bundle with fspy via Tracking struct; materialize .node addon on first use - consume runner-aware tool reports for cache decisions: * disableCache() short-circuits via ToolRequested * ignoreInput / ignoreOutput filter fspy reads/writes * tracked: true env / env-glob records folded into PostRunFingerprint - IPC server failure surfaces via IpcServerError; cache update is skipped - schema bumped to user_version = 13 (CacheEntryKey carries output_config, CacheEntryValue carries output_archive + tracked envs) Conflicts resolved against post-rebase main (#352 cfg(fspy) gating, #321 output archiving, input-negative reads-only filter): TrackingOutcome post-run summary preserved alongside Tracking pre-run handle; auto-input reads gated on input_config.includes_auto and filtered by input negatives + ignoreInput; auto-output writes filtered by negatives + ignoreOutput. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6198f6b to
c63db22
Compare
ffca388 to
53d7bd1
Compare
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53d7bd1883
ℹ️ 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".
- 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) <noreply@anthropic.com>
ef023c1 to
7ba7e0a
Compare
`index.js` becomes plain JS (no JSDoc type annotations), and the type declarations move to a new `index.d.ts` referenced from `package.json`'s `types` field. Consumers like `vite` — whose strict tsconfig has `isolatedDeclarations: true` and so can't enable `allowJs` — pick up types from the `.d.ts` directly, without a separate generation step or ambient module declaration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the vite-task ↔ vitejs/vite PR relationship, why the vite-task-client-dist branch exists (pnpm subdir `&path:` doesn't round-trip through `pnpm install`), and where the investigation can resume — full test matrix, ruled-out knobs, and remaining angles. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The runner-aware integration now lives upstream in vitejs/vite PR #22453, so vite-task no longer needs to patch `vite` locally to wire the calls into `@voidzero-dev/vite-task-client`. Both the root and playground workspaces switch their `vite` catalog entry to the pkg.pr.new build of the upstream PR, removing `patches/vite.patch`, the `packageExtensions.vite` injection, and the dist-branch investigation doc that's no longer relevant. Until @voidzero-dev/vite-task-client is published to npm and the integration ships in a real vite release, the `vite` catalog tracks https://pkg.pr.new/vite@22453. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `.node` addon used to export its operations as top-level functions
backed by per-thread mutable state initialized in `#[napi(module_exports)]
init`. Switch to a `load()` factory that returns a `RunnerClient` JS
class instance:
* The addon: `#[napi] pub fn load() -> Result<RunnerClient>` connects
to the runner's IPC at call time and returns a `#[napi] struct
RunnerClient` whose `&self` methods (`ignore_input`, `ignore_output`,
`disable_cache`, `get_env`, `get_envs`) talk to the runner. No
module-level mutable state, no `#[napi(module_exports)]` init.
* The wrapper: `require(VP_RUN_NODE_CLIENT_PATH).load()` instead of
`require(VP_RUN_NODE_CLIENT_PATH)`. Wrapper API is unchanged.
The point of the indirection is evolution headroom: the addon can
accept an options argument later (e.g. a version field) and return a
differently-shaped addon for newer wrappers, without breaking the
contract for callers that pass nothing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add two ignored e2e cases that exercise the Vite Task hooks the upstream
vite PR (#22453) added in this branch:
* `vite_build_cache::vite_node_env_change_invalidates_cache` — runs
`vt run --cache build` three times, alternating NODE_ENV. The first
run captures NODE_ENV in the fingerprint; the second is a cache
hit; the third (`NODE_ENV=development`) misses with
`tracked env 'NODE_ENV' changed`.
* `vite_dev_disable_cache::vite_dev_disables_cache` — runs a tiny
`dev.mjs` that starts a Vite dev server on an ephemeral port
(`server.port = 0`) and immediately closes it. Vite's
`_createServer` calls `disableCache()`, so even though the process
exits 0 the runner never stores the run, and the next invocation
re-executes.
The new fixture also needs `vite` available in its staging
`node_modules`, so extend the `link_tools_packages` allowlist in the
e2e harness from a single name to a `matches!`-set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two CI-bound fixes for the previous two commits: * `pnpm-lock.yaml` was still recording `@voidzero-dev/vite-task-client@ 1d49f00…` (the SHA referenced by the previous `vite` tarball at `pkg.pr.new/vite@22453`). The new tarball declares `…#1aedd944fdc592585013102dc205abedcb6d7be4`, so `pnpm dedupe --check` in `Format and Check Deps` failed. Bump the recorded SHA so the workspace lockfile matches what consumers of the catalog'd vite get at install time. * `#[napi]` on a struct + methods + the `load()` factory expands into glue that calls `std::format!` (via `napi::check_status!`), which the workspace `.clippy.toml` disallows in favor of `vite_str::format!`. The macro output isn't ours to rewrite, so allow `clippy::disallowed_macros` at the napi crate root and extend the `#![expect(...)]` justification. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/snapshots.toml # crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/snapshots/output_globs___files_restored_on_cache_hit.md # crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/snapshots/output_globs___negative_excludes_files_from_archive.md # crates/vite_task_bin/tests/e2e_snapshots/fixtures/output_cache_test/snapshots/output_globs___old_archive_removed_on_rewrite.md # pnpm-lock.yaml
The previous merge took `origin/main`'s `output_cache_test/snapshots.toml` (which declares `output_globs___*` cases) but kept this branch's `vite-task.json` and orphan `*.md` snapshots from the old `30a97a2e` design (`auto-output`, `glob-output`, …). The result was a fixture whose tasks didn't match the cases pnpm asked the test runner to exercise, so the three `output_globs___*` jobs failed on every runner. Replace the fixture wholesale with `origin/main`'s version so cases, tasks, sources, and snapshots all describe the same scenarios. The runner-aware-tools branch never relied on the dropped auto/glob cases itself — they were artefacts of an earlier in-flight design that landed upstream as #375 with a different surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1784d37 to
18066e3
Compare
The runner-aware addon's e2e fixtures (`ipc_client_test`, `vite_build_cache`, `vite_dev_disable_cache`) all carry `platform = "unix"` with the note that on Windows CI the Node child aborts with `failed to start the persistent thread of the Interprocess linger pool: Access is denied` when the napi addon tears down. The panic comes from `interprocess::os::windows::named_pipe::stream::Drop` handing dirty pipes to a lazily-spawned background thread that calls `FlushFileBuffers` for graceful close. Windows CI containers refuse the `CreateThread` from inside Node's worker-thread finalizer with ACCESS_DENIED, the crate `expect`s on that path, and the panic kills the child. Add a `Drop` impl for `Client` that, on Windows, reaches into the underlying `DuplexPipeStream` via `local_socket::Stream::NamedPipe(_)` and calls `assume_flushed()` so the inner stream's `Drop` skips the limbo detour. The IPC is strictly request/response, so once we have read each response the server has already consumed every byte we sent — there is nothing to flush at close time anyway. The fix unblocks Windows CI for every test that goes through the addon. Drop `platform = "unix"` from all nine `[[e2e]]` cases that were gated on the bug; they remain `ignore = true` because they still need a pnpm-populated `node_modules` to run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
18066e3 to
0397d4d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Set up a IPC channel between vite-task and the processes it spawns, so the spawned tools can declare at runtime what they actually read, wrote, or cared about, and then vite-task uses that to decide what to fingerprint in the cache.
Design notes:
docs/runner-task-ipc/.Problems this PR solves
Every example below is exercised by
patches/vite.patch, which wiresvite buildinto the IPC through@voidzero-dev/vite-task-client.1. Dynamic tracked envs
Before: the user had to declare every relevant env in
vite-task.json, statically:{ "tasks": { "build": { "env": ["NODE_ENV", "VITE_*"], "cache": true } } }This duplicates knowledge the tool already has. Forgetting
NODE_ENVsilently skips cache invalidation on mode change.envPrefix-matching envs (VITE_*by default) get inlined into the bundle throughimport.meta.env.*— so changingenvPrefix: 'MYAPP_'invite.config.jswithout updatingvite-task.jsondrifts: the runner still tracksVITE_*while the build output is driven byMYAPP_*.After: the tool declares its envs at runtime, driven by its own config.
The
buildtask invite-task.jsonneeds noenv:at all. ChangingenvPrefixinvite.config.jsdynamically changes the set of envs the runner tracks, with zero config edits on the runner side.2. Exclude tool's cache dir from input/output
Vite stores pre-bundled deps under
node_modules/.vite/and bundled configs undernode_modules/.vite-temp/. Every build reads the cache metadata (to check staleness) and writes fresh entries when it isn't stale. Without intervention the runner sees:There is a workaround already in vite-plus: voidzero-dev/vite-plus#1096 plus its follow-up #1198 hardcode
!node_modules/.vite-temp/**,!node_modules/.vite/**/results.json, and!dist/**as negative input globs on everyvpsubcommand (build,test,pack). That's not good enough:.vite/, moved temp dir, new subcommand with its own transient files), vp has to ship a matching glob update. It's a lockstep coupling that design-wise shouldn't exist..nuxt/, SvelteKit's.svelte-kit/, Next's.next/), needs its own hand-maintained list — vp can't ship it generically.After:
The declaration lives with the tool that owns the directory. The dep cache is vite's private concern.
3. Exclude output from input when a tool clears the folder before writing it
vite buildcallsemptyDir(outDir)before writingdist/.emptyDirhas to read the directory entries to know what to delete — those reads look identical to genuine input reads. Sincedist/is also where vite writes its final output, the runner sees a read-write overlap on the same paths and refuses to cache.After:
Only the writes count. The pattern generalizes: any tool that wipes-then-writes the same directory needs to tell the runner "my enumeration reads aren't inputs."
What's in this PR
vite_task_ipc_shared): message types + serialization shared by both ends.vite_task_server+vite_task_client): async server, sync blocking client, tested Rust-to-Rust.fspyfor dylib embedding. (Landed onmainvia refactor: extract materialized_artifact crate out of fspy #344 asmaterialized_artifact.)vite_task_client_napi+@voidzero-dev/vite-task-clientJS wrapper (fetchEnvsingle-name +fetchEnvsglob, with dedupe against already-setprocess.env).serve()'s returned iterator.Test plan
vite_task_server/tests/integration.rs)ignore_input,ignore_output,fetch_env,fetch_envs_glob,disable_cachevite buildviapatches/vite.patch(vite_build_cachefixture): NODE_ENV-change invalidation,envPrefix-driven tracked-env set change,dist/write restoration on cache hit