Repository navigation
fix(cache): fingerprint only whether files in node_modules/.bin exist - #824
Merged
wan9chi merged 1 commit intoOct 12, 2026
Merged
Conversation
wan9chi
added this pull request to stack #826
October 9, 2026 06:06
wan9chi
force-pushed
the
claude/node-modules-bin-existence
branch
from
October 9, 2026 06:08
dc4a249 to
e93f942
Compare
fspy benchmarklinuxmacoswindows |
wan9chi
force-pushed
the
claude/node-modules-bin-existence
branch
from
October 12, 2026 04:32
e93f942 to
5421bbe
Compare
wan9chi
force-pushed
the
claude/node-modules-bin-existence
branch
from
October 12, 2026 05:26
5421bbe to
39ce61e
Compare
wan9chi
added a commit
that referenced
this pull request
Oct 12, 2026
## Motivation `vitest_browser_cache::vitest_browser_caches_inputs_and_restores_outputs` fails intermittently on Windows runners in the `Test (windows-ignored)` job: ``` Error: listen EACCES: permission denied ::1:63315 ``` 63315 is the fixed default port for Vitest browser mode's server. Windows reserves dynamic port ranges (Hyper-V/WinNAT excluded port ranges), and listening on a reserved port fails with `EACCES`. Vite only tries another port on `EADDRINUSE`, so the run fails, `dist/result.json` is never written, and the snapshot diverges. It depends on which runner the job lands on: it failed on #824 and #825 and passed on other runs of the same code. ## Changes The fixture's browser server now listens on port 0, so the OS chooses a free port. Vite supports `server.listen(0)` directly, but the config can't request it the normal way: - `test.api: { port: 0 }`: Vitest only copies `api.port` when it is truthy, so `0` falls back to 63315. - Setting `test.api` at all, even with a concrete port, adds an `API started at http://localhost:<port>/` banner line that would change the snapshot. `test.browser.api` prints a deprecation warning in Vitest 5. Instead, a small `enforce: 'post'` plugin sets `test.api.port = 0` after Vitest resolves `test.api`. The banner stays unchanged, and the snapshot is identical. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pnpm's bin shims embed the absolute workspace path in `NODE_PATH` and a `cmd-shim-target` comment, so any task that runs a dependency's executable missed after installing with the workspace at another path. What a shim runs is tracked through the package files the task reads, so the content of files directly inside `node_modules/.bin` is no longer hashed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wan9chi
force-pushed
the
claude/node-modules-bin-existence
branch
from
October 12, 2026 05:33
39ce61e to
ec0eba1
Compare
wan9chi
added a commit
that referenced
this pull request
Oct 13, 2026
#850) ## Motivation #823 hashes pnpm's `node_modules/.modules.yaml` without `prunedAt`, `storeDir`, `virtualStoreDir` and `linkedSkills`. What's left protects little and still causes needless misses: - Every cached `vite build` and `vitest` task reads the file because Vite reads it while resolving its config ([`packages/vite/src/node/server/index.ts`](https://github.com/vitejs/vite/blob/dce9fe69d37e957b62f3b5df61a761f7f2f949f5/packages/vite/src/node/server/index.ts#L1332-L1361)). Vite reads only `virtualStoreDir`, to add pnpm's global virtual store to the dev server's `server.fs.allow`, inside a try/catch for non-pnpm projects. `virtualStoreDir` is already left out of the hash, so the hash protects nothing Vite uses, and for `vite build` the file doesn't affect the output. - The other fields describe `node_modules`, which tasks already read. In pnpm 11 they are `hoistedDependencies`, `hoistPattern`, `publicHoistPattern`, `hoistedLocations`, `nodeLinker`, `included`, `skipped`, `injectedDeps`, `layoutVersion`, `virtualStoreDirMaxLength`, `pendingBuilds`, `ignoredBuilds`, `allowBuilds`, `packageManager` and `virtualStoreOnly`. When they change in a way that matters, the files a task reads in `node_modules` change too, and those reads are tracked. - Some of the hashed fields still vary across machines, so tasks missed with `'node_modules/.modules.yaml' modified`: - `packageManager`: the version of the pnpm that installed - `pendingBuilds`: filled after `pnpm install --ignore-scripts` - `virtualStoreOnly`: set after `pnpm fetch` - `skipped`: differs between glibc and musl for platform packages under the same cache key - The list of fields to leave out has to track pnpm's format. ## Changes - `node_modules/.modules.yaml` is now fingerprinted only by whether it exists, the same treatment #824 gives files in `node_modules/.bin`. A missing manifest still differs from an existing one, since tools detect pnpm by the file existing (Vite's try/catch, which-pm). - Trade-off: a task whose output depends on these fields without any file it reads in `node_modules` differing no longer misses. The examples #823 named are LavaMoat's allow-scripts check (`pendingBuilds`/`ignoredBuilds`), which-pm's pnpm version, and Nx's `hoistedDependencies`. These rarely run inside cached vp tasks, and in realistic cases `node_modules` differs too. - The JSON/YAML parsing, the canonical JSON writer and the fallback for unparsable content are removed, along with `vt`'s `serde_norway` dependency. Both files are hashed as empty content without being read, which is the hash files in `node_modules/.bin` already had, so their cache entries stay valid. - The `pnpm_modules_manifest` e2e fixture now shows that changing the manifest's content keeps the cache hit and removing it misses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Motivation
pnpm's shims in
node_modules/.binembed the absolute workspace path, inNODE_PATHand in a trailing# cmd-shim-target=comment. Almost every task runs a dependency's executable through one, so installing with the workspace at another path, such as another clone or a CI checkout, made those tasks miss with'node_modules/.bin/tsc' modified.Changes
node_modules/.bindirectory are now fingerprinted only by whether they exist. A missing shim still differs from an existing one.exectarget is a relative path, and only pnpm readscmd-shim-target.C:\ws,C:/ws,/mnt/c/ws) that rewriting the absolute paths in place would. It also covers.cmdand.ps1shims, pnpm's global virtual store, and npm/Yarn symlinks.NODE_PATHwas an incidental signal that a dependency changed, since reads in the store are outside the workspace and untracked. Tracking store reads properly is a separate problem.🤖 Generated with Claude Code