Skip to content

fix(cache): fingerprint only whether node_modules/.modules.yaml exists - #850

Merged
wan9chi merged 1 commit into
mainfrom
fix/modules-yaml-existence-only
Oct 13, 2026
Merged

wan9chi merged 1 commit into
mainfrom
fix/modules-yaml-existence-only

Conversation

@wan9chi

@wan9chi wan9chi commented Oct 13, 2026

Copy link
Copy Markdown
Member

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). 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 fix(cache): fingerprint only whether files in node_modules/.bin exist #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 fix(cache): leave noisy fields of pnpm's .modules.yaml out of its fingerprint #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

Hashing a redacted copy of pnpm's modules manifest still missed the cache when fields that vary across machines changed, such as packageManager, pendingBuilds, virtualStoreOnly and skipped. Vite, which reads the file in every cached vite build and vitest task, uses only virtualStoreDir, which was already left out. The other fields describe node_modules, whose files tasks read and are tracked.

Fingerprint only whether the file exists, like files in node_modules/.bin, and remove the JSON/YAML parsing and the redaction machinery it needed, including vt's serde_norway dependency.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi force-pushed the fix/modules-yaml-existence-only branch from 2f156fb to f465a92 Compare October 13, 2026 07:26
@github-actions

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  +0.42%  [ -4.22% ..  +5.12%]  overhead   +50.30%
dynamic/access             change  -0.44%  [ -1.96% ..  +1.25%]  overhead   +10.42%
dynamic/access-relative    change  +0.77%  [ -1.08% .. +16.45%]  overhead   +62.90%
dynamic/access-contended   change  +4.84%  [-12.03% .. +14.43%]  overhead   +24.13%
static/launch              change  +2.87%  [ -7.40% .. +11.57%]  overhead  +186.86%
static/access              change  +2.64%  [-10.05% .. +20.09%]  overhead  +895.06%
static/access-relative     change  -0.97%  [ -4.73% ..  +3.08%]  overhead +1377.39%
static/access-contended    change  +0.09%  [ -9.38% .. +12.48%]  overhead +2092.14%

macos

dynamic/launch             change  -0.43%  [ -2.87% ..  +2.98%]  overhead  +220.86%
dynamic/access             change  -0.40%  [ -4.14% ..  +2.81%]  overhead    +5.88%
dynamic/access-relative    change  -0.22%  [ -2.19% ..  +1.76%]  overhead  +272.91%
dynamic/access-contended   change  +0.28%  [ -3.05% ..  +2.38%]  overhead    +4.03%

windows

dynamic/launch             change  +1.34%  [ -7.35% ..  +7.85%]  overhead   +28.92%
dynamic/access             change  -0.18%  [ -3.87% ..  +4.46%]  overhead    +2.19%
dynamic/access-relative    change  +0.35%  [ -3.66% ..  +8.45%]  overhead    +2.14%
dynamic/access-contended   change  -0.80%  [ -3.70% ..  +3.46%]  overhead    +1.90%

@wan9chi
wan9chi merged commit 1ba1e46 into main Oct 13, 2026
32 of 34 checks passed
@wan9chi
wan9chi deleted the fix/modules-yaml-existence-only branch October 13, 2026 07:41
cursor Bot added a commit to voidzero-dev/vite-plus that referenced this pull request Oct 21, 2026
Bumps vite-task from `9ba4cc9` to `8db455e`.

Changelog:
voidzero-dev/vite-task@9ba4cc9...8db455e#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4ed

## Breaking change adapted

fspy no longer reports a bitset of access modes on each path. Each
tracked operation is its own `PathAccess` (`Read`, `Write`, `ReadWrite`,
or `ReadDir`). `vp_command` combines the operations that touch the same
workspace-relative path into the existing `AccessMode` set, so the
JavaScript `run_command` result is unchanged.

## User-facing vite-task changes

- A mistyped task setting now fails with an error that names the field,
instead of `data did not match any variant of untagged enum
UserTaskDefinition` (voidzero-dev/vite-task#815).
- `vp run` no longer panics on scripts that contain non-ASCII
characters, and no longer cuts those commands off where it displays them
(voidzero-dev/vite-task#861).
- Reinstalling with another pnpm version no longer changes the cache
fingerprint of `node_modules/.modules.yaml`; only whether the file
exists is hashed (voidzero-dev/vite-task#850).
- Cached tasks on Linux and macOS no longer abort when a program passes
`-1` as a directory descriptor, and concurrent syscalls in tracked
programs no longer overflow the stack (voidzero-dev/vite-task#862,
voidzero-dev/vite-task#854).

Docs are unchanged. These are fixes to existing behavior, and the guides
do not describe the old error text or the `.modules.yaml` fingerprint.

## Validation

- `cargo check --workspace --all-features --all-targets --locked`
- `cargo clippy -p vp_command -p vite-plus-cli --all-targets
--all-features -- --deny warnings`
- `cargo test` for `vp_command`, `vp_error`, `vp_pm_cli`,
`vp_js_runtime`, `vp_migration`, `vp_shared`, `vp_static_config`,
`vite-plus-cli`, and `vp_global_cli`


<div><a
href="https://cursor.com/agents/bc-9c9cb178-3b56-4d68-9556-3028d946a3ec?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/automations/3a586a0e-cc2b-11f1-bec7-b2318963e0b6"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/view-automation-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/view-automation-light.png"><img
alt="View Automation" width="141" height="28"
src="https://cursor.com/assets/images/view-automation-dark.png"></picture></a>&nbsp;</div>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Charles Wang <wan9chi@users.noreply.github.com>
Sign up for free to 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