Skip to content

fix(cache): leave noisy fields of pnpm's .modules.yaml out of its fingerprint - #823

Merged
wan9chi merged 1 commit into
mainfrom
claude/modules-yaml-issues-604e1f
Oct 12, 2026
Merged

wan9chi merged 1 commit into
mainfrom
claude/modules-yaml-issues-604e1f

Conversation

@wan9chi

@wan9chi wan9chi commented Oct 9, 2026

Copy link
Copy Markdown
Member

Motivation

pnpm's node_modules/.modules.yaml is read by every cached vite build and vitest task, because Vite's resolveConfig reads it to extend the dev server's fs.allow (vite-plus#2845). pnpm rewrites prunedAt on fresh installs and records machine-specific paths, so those tasks missed the cache after every reinstall with 'node_modules/.modules.yaml' modified, and could never hit across machines.

Changes

  • The manifest is hashed as canonical JSON without the fields that vary with time or with the machine:
    • prunedAt: when pnpm last pruned the virtual store
    • storeDir: absolute path of pnpm's store
    • virtualStoreDir: absolute on Windows; inside the store with pnpm's global virtual store
    • linkedSkills: absolute for skills outside the workspace
  • Every other field is still hashed, because tools read them: Nx reads hoistedDependencies, LavaMoat reads pendingBuilds/ignoredBuilds, and which-pm reads packageManager. Whether the file exists is fingerprinted as before.
  • The file is parsed as JSON first, then as YAML, since pnpm < 10.29 writes YAML. JSON comes first because it parses about 5× faster, and because the YAML parser rejects keys over 1024 characters, which long dependency paths exceed. Content in neither format is hashed as-is.
  • Object keys are sorted before hashing. The serialized bytes are streamed into xxHash3 through a small io::Write adapter, so no intermediate string is built. std::hash::Hash isn't used because its output isn't stable across platforms or Rust versions.
  • Redactions live in session/execute/redact/, one module per file, each with a matches condition and a redact action called from hash_file_content.

🤖 Generated with Claude Code

@wan9chi
wan9chi added this pull request to stack #826 October 9, 2026 06:06
@wan9chi
wan9chi force-pushed the claude/modules-yaml-issues-604e1f branch from 3713d39 to 75c9721 Compare October 9, 2026 06:08
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  +0.89%  [ -7.19% .. +12.44%]  overhead   +56.26%
dynamic/access             change  -0.84%  [ -7.20% ..  +6.81%]  overhead    +5.26%
dynamic/access-relative    change  +0.71%  [ -6.50% ..  +6.82%]  overhead   +51.73%
dynamic/access-contended   change  +0.00%  [ -4.06% ..  +3.81%]  overhead   +10.54%
static/launch              change  +2.66%  [ -5.03% .. +10.70%]  overhead  +170.47%
static/access              change  -0.34%  [-15.11% .. +10.66%]  overhead  +765.17%
static/access-relative     change  +0.07%  [ -5.17% ..  +6.73%]  overhead +1022.96%
static/access-contended    change  +1.54%  [ -5.21% ..  +8.04%]  overhead +2480.21%

macos

dynamic/launch             change  +0.45%  [ -2.59% ..  +3.45%]  overhead  +213.73%
dynamic/access             change  -0.36%  [ -2.80% ..  +2.20%]  overhead    +4.32%
dynamic/access-relative    change  -0.22%  [ -3.26% ..  +3.73%]  overhead  +274.61%
dynamic/access-contended   change  +0.47%  [ -4.32% ..  +5.50%]  overhead    +5.95%

windows

dynamic/launch             change  -0.03%  [ -1.90% ..  +2.35%]  overhead   +25.36%
dynamic/access             change  +0.00%  [ -0.59% ..  +0.96%]  overhead    +0.99%
dynamic/access-relative    change  +0.37%  [ -0.92% ..  +0.94%]  overhead    +1.13%
dynamic/access-contended   change  +0.19%  [ -0.56% ..  +1.28%]  overhead    +1.27%

@wan9chi
wan9chi force-pushed the claude/modules-yaml-issues-604e1f branch 2 times, most recently from 7c05e67 to 341fdda Compare October 12, 2026 05:25
…gerprint

pnpm rewrites `node_modules/.modules.yaml` with a new `prunedAt` on fresh
installs, and records machine-specific paths in it. Vite reads it on every
`resolveConfig`, so cached `vite build` and `vitest` tasks missed after
every reinstall and never hit across machines.

The manifest is now parsed (JSON, falling back to YAML for pnpm < 10.29)
and hashed as canonical JSON without `prunedAt`, `storeDir`,
`virtualStoreDir`, and `linkedSkills`. Content in neither format is hashed
as-is. Redactions live in `session/execute/redact/`, one module per file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi force-pushed the claude/modules-yaml-issues-604e1f branch from 341fdda to 6ee27ab Compare October 12, 2026 05:33
@wan9chi
wan9chi merged commit 4e663e7 into main Oct 12, 2026
20 checks passed
@wan9chi
wan9chi deleted the claude/modules-yaml-issues-604e1f branch October 12, 2026 06:31
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>
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