perf: record locally-resolved lockfile in verification cache (#11714)

The lockfile verification cache currently only records the lockfile that exists at the **start** of an install. So a flow like:

```
pnpm install <pkg>
rm -rf node_modules
pnpm install
```

re-runs the per-package registry round-trip against the newly written lockfile, even though the local resolver already enforced the policy when picking those versions. The fresh lockfile is now recorded immediately after each install-time write, so the second install takes the cache fast path.

## Implementation

### Recording the post-resolution lockfile

- New helper `recordLockfileVerified` (in `installing/deps-installer/src/install/`). Gated on `cacheDir` + non-empty `resolutionVerifiers` — same gate the pre-resolution verifier uses.
- Two thin combiners over the lockfile writers: `writeWantedLockfileAndRecordVerified` and `writeLockfilesAndRecordVerified`. The install paths use these so the record always runs alongside the write.

### Hash stability: writer returns the canonical lockfile

The cache stores `hashObject(LockfileObject)` and the next install computes the same hash off the file it loads from disk. For the hashes to match, both ends must compute over structurally identical objects. They don't, naïvely: the in-memory write object can carry `undefined` optional fields (e.g. `settings.dedupePeers = undefined` from `opts.dedupePeers || undefined` in install code) that YAML drops on serialize — `object-hash` treats undefined vs missing as distinct values.

- `writeWantedLockfile` / `writeLockfiles` (in `@pnpm/lockfile.fs`) now return the canonical post-write `LockfileObject`: `convertToLockfileObject(stripUndefinedDeep(lockfileFile))`. The strip walks the existing object graph in memory rather than going through a `yaml.load` round-trip, so non-cache callers (deploy, deps-restorer, make-dedicated-lockfile, agent server) pay near-zero cost.
- Install hooks hash the writer's returned value, not the raw in-memory input. Guaranteed by construction to match what the next reader produces.

### `useGitBranchLockfile` correctness

The pre-resolution verification gate and the new post-write recorder were both keying cache records on a hard-coded `pnpm-lock.yaml`. Under `useGitBranchLockfile` the actual file is `pnpm-lock.<branch>.yaml`, so the stat shortcut hit `ENOENT` and the cache effectively never engaged for git-branch users. Both sites now resolve the real filename via `getWantedLockfileName`. The wrappers compute it once and pass it to the writer via a new optional `lockfileName` opt so `useGitBranchLockfile` installs don't fork `getCurrentBranch` twice per write.

### Bug fix unrelated to the cache, found during review

`writeLockfiles`' differs branch was deciding whether to remove or keep `node_modules/.pnpm/lock.yaml` based on `isEmptyLockfile(wantedLockfile)`. Filtered-current callers (deps-restorer) pass an empty current against a non-empty wanted, so this could leave a stale current lockfile on disk. Fixed to key off the current.

### Comments policy

`AGENTS.md` (and `pacquet/AGENTS.md`) now spell out the comment defaults: write self-documenting code, do not restate at call sites what the callee's JSDoc / doc comment already says, comments are reserved for the non-obvious *why*. The pruning pass in this PR brings the changed code in line.

## API surface

- `@pnpm/lockfile.fs` (minor):
  - `writeWantedLockfile`: return widened from `Promise<void>` to `Promise<LockfileObject>`. New optional `lockfileName` opt.
  - `writeCurrentLockfile`: return widened to `Promise<LockfileObject | undefined>` (undefined when the empty-lockfile branch unlinks).
  - `writeLockfiles`: return widened from `Promise<void>` to `Promise<{ wantedLockfile, currentLockfile }>`. New optional `wantedLockfileName` opt. New exported `WriteLockfilesResult` type.
  - New export: `getWantedLockfileName`.
- `@pnpm/installing.deps-installer` (patch): internal-only wrappers; no external API change.
This commit is contained in:
Zoltan Kochan
2026-05-18 14:55:16 +02:00
committed by GitHub
parent e2a3c68309
commit 2a9bd897bf
14 changed files with 588 additions and 19 deletions

View File

@@ -0,0 +1,7 @@
---
"@pnpm/installing.deps-installer": patch
"@pnpm/lockfile.fs": minor
"pnpm": patch
---
Record the post-resolution lockfile in the verification cache. Previously the cache only captured the lockfile that was loaded at the start of an install, so a flow like `pnpm install <pkg>` followed by `rm -rf node_modules && pnpm install` re-ran the per-package registry round-trip against the newly written lockfile even though the local resolver had already enforced the policy when picking those versions. The fresh lockfile is now recorded immediately after each install-time write, so the second install takes the cache fast path.

View File

@@ -186,6 +186,23 @@ To ensure your code adheres to the style guide, run:
pnpm run lint
```
### Comments
Write code that explains itself. A reader should understand what a function does from its name, parameters, and types — not from prose above the call site.
Defaults:
- **Do not write a comment** that restates what the code already says. If renaming a variable, splitting a helper, or moving a check to a more obvious place would carry the information, do that instead.
- **Do not repeat documentation** at call sites that already lives on the callee. If the function has a JSDoc, the call site shouldn't re-explain what calling it does. Update the JSDoc once; let every call site benefit.
- **JSDoc is for the function's contract** — preconditions, postconditions, edge cases, why the function exists. Not for re-narrating the body.
Write a comment only when:
- The reason for the code is non-obvious from reading it (a hidden invariant, a workaround for a known bug, a deliberate exception to the surrounding pattern).
- The right name doesn't fit — e.g., a temporary technical constraint that's worth flagging but doesn't justify a new symbol.
Before adding a comment, ask: "Could I rename, restructure, or extract instead?" If yes, do that. The bar for prose-in-code is high; the bar for prose-that-restates-code is "don't."
## Common Gotchas
### Error Type Checking in Jest (TypeScript only)

View File

@@ -42,6 +42,7 @@ import { writeModulesManifest } from '@pnpm/installing.modules-yaml'
import {
type CatalogSnapshots,
cleanGitBranchLockfiles,
getWantedLockfileName,
type LockfileObject,
type ProjectSnapshot,
readWantedLockfile,
@@ -97,6 +98,8 @@ import { linkPackages } from './link.js'
import { reportPeerDependencyIssues } from './reportPeerDependencyIssues.js'
import { validateModules } from './validateModules.js'
import { verifyLockfileResolutions } from './verifyLockfileResolutions.js'
import { writeLockfilesAndRecordVerified } from './writeLockfilesAndRecordVerified.js'
import { writeWantedLockfileAndRecordVerified } from './writeWantedLockfileAndRecordVerified.js'
class LockfileConfigMismatchError extends PnpmError {
constructor (outdatedLockfileSettingName: string) {
@@ -358,10 +361,17 @@ export async function mutateModules (
// resolver's own filters already cover fresh resolution. We run this
// exactly once, right after the lockfile is loaded from disk, before any
// path branches.
const cacheActive = opts.cacheDir != null && opts.resolutionVerifiers.length > 0
const wantedLockfilePath = cacheActive
? path.resolve(ctx.lockfileDir, await getWantedLockfileName({
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
}))
: undefined
try {
await verifyLockfileResolutions(ctx.wantedLockfile, opts.resolutionVerifiers, {
cacheDir: opts.cacheDir,
lockfilePath: path.resolve(ctx.lockfileDir, WANTED_LOCKFILE),
lockfilePath: wantedLockfilePath,
})
} catch (err) {
// verifyLockfileResolutions is the one throw site in this function
@@ -1657,11 +1667,13 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
const currentLockfileDir = path.join(ctx.rootModulesDir, '.pnpm')
await Promise.all([
opts.useLockfile && opts.saveLockfile
? writeLockfiles({
? writeLockfilesAndRecordVerified({
currentLockfile: result.currentLockfile,
currentLockfileDir,
wantedLockfile: newLockfile,
wantedLockfileDir: ctx.lockfileDir,
cacheDir: opts.cacheDir,
resolutionVerifiers: opts.resolutionVerifiers,
...lockfileOpts,
})
: writeCurrentLockfile(ctx.virtualStoreDir, result.currentLockfile),
@@ -1716,7 +1728,13 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
}
} else {
if (opts.useLockfile && opts.saveLockfile && !isInstallationOnlyForLockfileCheck) {
await writeWantedLockfile(ctx.lockfileDir, newLockfile, lockfileOpts)
await writeWantedLockfileAndRecordVerified({
lockfileDir: ctx.lockfileDir,
lockfile: newLockfile,
cacheDir: opts.cacheDir,
resolutionVerifiers: opts.resolutionVerifiers,
...lockfileOpts,
})
}
if (opts.nodeLinker !== 'hoisted') {
@@ -2264,7 +2282,14 @@ async function installFromPnpmRegistry (
storeIndex.close()
}
await writeWantedLockfile(lockfileDir, lockfile)
await writeWantedLockfileAndRecordVerified({
lockfileDir,
lockfile,
cacheDir: opts.cacheDir,
resolutionVerifiers: opts.resolutionVerifiers,
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
})
logger.info({
message: `Resolved ${agentStats.totalPackages} packages: ${agentStats.alreadyInStore} cached, ${agentStats.filesToDownload} files to download`,

View File

@@ -0,0 +1,36 @@
import { hashObject } from '@pnpm/crypto.object-hasher'
import type { LockfileObject } from '@pnpm/lockfile.fs'
import type { ResolutionVerifier } from '@pnpm/resolving.resolver-base'
import { recordVerification } from './verifyLockfileResolutionsCache.js'
export interface RecordLockfileVerifiedOptions {
cacheDir?: string
/** Absolute path of the lockfile the next install will read.
* Under `useGitBranchLockfile` this is the branch-suffixed name. */
lockfilePath: string
/** The writer's canonical return value — see {@link writeWantedLockfile}.
* Passing the raw in-memory write object would record a hash the
* next install can't match (YAML drops undefined fields). */
lockfile: LockfileObject
resolutionVerifiers: readonly ResolutionVerifier[] | undefined
}
/**
* Records the post-resolution lockfile as verified so the next install
* skips the registry round-trip. Skipping is safe: fresh local picks
* are filtered by the resolver (see
* `resolving/npm-resolver/src/pickPackage.ts`) and carried-over entries
* already passed the gate at the top of `mutateModules`, so the
* recorded lockfile is policy-clean by construction.
*/
export function recordLockfileVerified (opts: RecordLockfileVerifiedOptions): void {
if (!opts.cacheDir) return
if (!opts.resolutionVerifiers?.length) return
if (!opts.lockfile.packages) return
recordVerification(opts.cacheDir, {
lockfilePath: opts.lockfilePath,
verifiers: opts.resolutionVerifiers,
hashLockfile: () => hashObject(opts.lockfile),
})
}

View File

@@ -97,8 +97,7 @@ export async function verifyLockfileResolutions (
// miss the precomputed stat+hash flow to recordVerification.
type Precomputed = ReturnType<typeof tryLockfileVerificationCache>['precomputed']
let cachePrecomputed: Precomputed | undefined
// hashObject is streaming (no "Invalid string length" on huge lockfiles)
// and key-order-stable, which JSON.stringify is not.
// hashObject streams and is key-order-stable, unlike JSON.stringify.
let cachedHash: string | undefined
const hashLockfile = (): string => {
if (cachedHash == null) cachedHash = hashObject(lockfile)

View File

@@ -0,0 +1,48 @@
import path from 'node:path'
import { getWantedLockfileName, type LockfileObject, writeLockfiles, type WriteLockfilesResult } from '@pnpm/lockfile.fs'
import type { ResolutionVerifier } from '@pnpm/resolving.resolver-base'
import { recordLockfileVerified } from './recordLockfileVerified.js'
export interface WriteLockfilesAndRecordVerifiedOptions {
wantedLockfile: LockfileObject
wantedLockfileDir: string
currentLockfile: LockfileObject
currentLockfileDir: string
useGitBranchLockfile?: boolean
mergeGitBranchLockfiles?: boolean
cacheDir?: string
resolutionVerifiers: readonly ResolutionVerifier[] | undefined
}
/** Plural counterpart of {@link writeWantedLockfileAndRecordVerified}. */
export async function writeLockfilesAndRecordVerified (
opts: WriteLockfilesAndRecordVerifiedOptions
): Promise<WriteLockfilesResult> {
const cacheActive = opts.cacheDir != null && (opts.resolutionVerifiers?.length ?? 0) > 0
const wantedLockfileName = cacheActive
? await getWantedLockfileName({
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
})
: undefined
const written = await writeLockfiles({
wantedLockfile: opts.wantedLockfile,
wantedLockfileDir: opts.wantedLockfileDir,
currentLockfile: opts.currentLockfile,
currentLockfileDir: opts.currentLockfileDir,
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
wantedLockfileName,
})
if (cacheActive) {
recordLockfileVerified({
cacheDir: opts.cacheDir,
lockfilePath: path.resolve(opts.wantedLockfileDir, wantedLockfileName!),
lockfile: written.wantedLockfile,
resolutionVerifiers: opts.resolutionVerifiers,
})
}
return written
}

View File

@@ -0,0 +1,42 @@
import path from 'node:path'
import { getWantedLockfileName, type LockfileObject, writeWantedLockfile } from '@pnpm/lockfile.fs'
import type { ResolutionVerifier } from '@pnpm/resolving.resolver-base'
import { recordLockfileVerified } from './recordLockfileVerified.js'
export interface WriteWantedLockfileAndRecordVerifiedOptions {
lockfileDir: string
lockfile: LockfileObject
cacheDir?: string
resolutionVerifiers: readonly ResolutionVerifier[] | undefined
useGitBranchLockfile?: boolean
mergeGitBranchLockfiles?: boolean
}
/** Combines {@link writeWantedLockfile} and {@link recordLockfileVerified} — see each for semantics. */
export async function writeWantedLockfileAndRecordVerified (
opts: WriteWantedLockfileAndRecordVerifiedOptions
): Promise<LockfileObject> {
const cacheActive = opts.cacheDir != null && (opts.resolutionVerifiers?.length ?? 0) > 0
const lockfileName = cacheActive
? await getWantedLockfileName({
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
})
: undefined
const written = await writeWantedLockfile(opts.lockfileDir, opts.lockfile, {
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
lockfileName,
})
if (cacheActive) {
recordLockfileVerified({
cacheDir: opts.cacheDir,
lockfilePath: path.resolve(opts.lockfileDir, lockfileName!),
lockfile: written,
resolutionVerifiers: opts.resolutionVerifiers,
})
}
return written
}

View File

@@ -0,0 +1,214 @@
import fs from 'node:fs'
import os from 'node:os'
import path from 'node:path'
import { afterEach, beforeEach, expect, test } from '@jest/globals'
import { WANTED_LOCKFILE } from '@pnpm/constants'
import { hashObject } from '@pnpm/crypto.object-hasher'
import { type LockfileObject, readWantedLockfile, writeWantedLockfile } from '@pnpm/lockfile.fs'
import type { ResolutionVerifier } from '@pnpm/resolving.resolver-base'
import { recordLockfileVerified } from '../../src/install/recordLockfileVerified.js'
import { tryLockfileVerificationCache } from '../../src/install/verifyLockfileResolutionsCache.js'
let tmpDir!: string
let cacheDir!: string
let lockfileDir!: string
let lockfilePath!: string
beforeEach(async () => {
tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-record-verified-'))
cacheDir = path.join(tmpDir, 'cache')
lockfileDir = path.join(tmpDir, 'project')
lockfilePath = path.resolve(lockfileDir, WANTED_LOCKFILE)
await fs.promises.mkdir(lockfileDir, { recursive: true })
})
afterEach(async () => {
await fs.promises.rm(tmpDir, { recursive: true, force: true })
})
function mraVerifier (current: number): ResolutionVerifier {
return {
policy: { minimumReleaseAge: current },
canTrustPastCheck: (cached) => {
const past = cached.minimumReleaseAge
return typeof past === 'number' && past >= current
},
verify: async () => ({ ok: true }),
}
}
function makeLockfile (): LockfileObject {
return {
lockfileVersion: '9.0',
importers: {
'.': {
specifiers: { 'is-positive': '^1.0.0' },
dependencies: { 'is-positive': '1.0.0' },
},
},
packages: {
'is-positive@1.0.0': {
resolution: {
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
tarball: '',
},
},
},
} as unknown as LockfileObject
}
test('no-op when cacheDir is undefined', () => {
recordLockfileVerified({
cacheDir: undefined,
lockfilePath,
lockfile: makeLockfile(),
resolutionVerifiers: [mraVerifier(60)],
})
expect(fs.existsSync(cacheDir)).toBe(false)
})
test('no-op when resolutionVerifiers is empty', () => {
recordLockfileVerified({
cacheDir,
lockfilePath,
lockfile: makeLockfile(),
resolutionVerifiers: [],
})
expect(fs.existsSync(cacheDir)).toBe(false)
})
test('no-op when resolutionVerifiers is undefined', () => {
recordLockfileVerified({
cacheDir,
lockfilePath,
lockfile: makeLockfile(),
resolutionVerifiers: undefined,
})
expect(fs.existsSync(cacheDir)).toBe(false)
})
test('records nothing when the in-memory lockfile has no packages', async () => {
await writeWantedLockfile(lockfileDir, makeLockfile())
recordLockfileVerified({
cacheDir,
lockfilePath,
lockfile: { lockfileVersion: '9.0', importers: {} } as unknown as LockfileObject,
resolutionVerifiers: [mraVerifier(60)],
})
expect(fs.existsSync(path.join(cacheDir, 'lockfile-verified.jsonl'))).toBe(false)
})
test('records the load-equivalent hash — matches what the next install computes off-disk', async () => {
// Use a fixture that carries an explicit `undefined` optional field
// (the real divergence case install-time code produces), then pass
// the *writer's return value* to recordLockfileVerified — same flow
// as the production call sites. Passing the in-memory input here
// instead would silently regress the moment the writer's
// canonicalization stops matching the reader's output.
const inMemoryLockfile = {
...makeLockfile(),
settings: {
autoInstallPeers: true,
excludeLinksFromLockfile: false,
dedupePeers: undefined,
},
} as unknown as LockfileObject
const written = await writeWantedLockfile(lockfileDir, inMemoryLockfile)
recordLockfileVerified({
cacheDir,
lockfilePath,
lockfile: written,
resolutionVerifiers: [mraVerifier(60)],
})
// The cache contract: the next install hashes its loaded
// `LockfileObject` and looks the hash up. The recorded hash must
// match what that lookup computes.
const loaded = await readWantedLockfile(lockfileDir, { ignoreIncompatible: false })
expect(loaded).not.toBeNull()
const expectedHash = hashObject(loaded!)
const cacheFile = path.join(cacheDir, 'lockfile-verified.jsonl')
const record = JSON.parse(fs.readFileSync(cacheFile, 'utf8').trim()) as {
lockfile: { hash: string, path: string }
}
expect(record.lockfile.hash).toBe(expectedHash)
expect(record.lockfile.path).toBe(lockfilePath)
})
test('respects the caller-supplied lockfilePath — git-branch lockfiles record under their branch-suffixed filename', async () => {
// Simulates `useGitBranchLockfile`: the actual on-disk lockfile is
// pnpm-lock.<branch>.yaml, not pnpm-lock.yaml. The helper has no
// git logic of its own — it records whatever path the caller hands
// it, so cache lookups on the same path will hit.
const branchLockfilePath = path.resolve(lockfileDir, 'pnpm-lock.feature-x.yaml')
await fs.promises.writeFile(branchLockfilePath, 'lockfileVersion: \'9.0\'\n')
const lockfile = makeLockfile()
recordLockfileVerified({
cacheDir,
lockfilePath: branchLockfilePath,
lockfile,
resolutionVerifiers: [mraVerifier(60)],
})
const cacheFile = path.join(cacheDir, 'lockfile-verified.jsonl')
const record = JSON.parse(fs.readFileSync(cacheFile, 'utf8').trim()) as {
lockfile: { path: string }
}
expect(record.lockfile.path).toBe(branchLockfilePath)
})
test('records a cache entry that the next install hits on both the stat shortcut and hash fallback paths', async () => {
// Mirror real call sites: hand `recordLockfileVerified` the
// writer's return value rather than the in-memory input. With an
// explicit `undefined` optional field in the fixture, those two
// diverge structurally — the in-memory variant would record a hash
// the next install can't match, and this test would silently miss
// that regression.
const inMemoryLockfile = {
...makeLockfile(),
settings: {
autoInstallPeers: true,
excludeLinksFromLockfile: false,
dedupePeers: undefined,
},
} as unknown as LockfileObject
const written = await writeWantedLockfile(lockfileDir, inMemoryLockfile)
recordLockfileVerified({
cacheDir,
lockfilePath,
lockfile: written,
resolutionVerifiers: [mraVerifier(60)],
})
const loaded = (await readWantedLockfile(lockfileDir, { ignoreIncompatible: false }))!
// Stat shortcut: file untouched since record.
const statResult = tryLockfileVerificationCache(cacheDir, {
lockfilePath,
verifiers: [mraVerifier(60)],
hashLockfile: () => hashObject(loaded),
})
expect(statResult.hit).toBe(true)
// Hash fallback: invalidate stat fields the cache compares against so the
// shortcut bails. This is the CI-checkout / new-worktree path; the hash
// has to match for the fallback to hit, which is the whole point of
// hashing the canonical load-equivalent form. Use `size = -1` (impossible
// for a real file) rather than zeroing `inode`/`mtimeNs` alone — on
// Windows `stat.ino` is often 0, which would let the cached record
// accidentally match and skip the fallback path we want to exercise.
const cacheFile = path.join(cacheDir, 'lockfile-verified.jsonl')
const cached = JSON.parse(fs.readFileSync(cacheFile, 'utf8').trim()) as {
lockfile: { size: number }
}
cached.lockfile.size = -1
fs.writeFileSync(cacheFile, `${JSON.stringify(cached)}\n`)
const hashResult = tryLockfileVerificationCache(cacheDir, {
lockfilePath,
verifiers: [mraVerifier(60)],
hashLockfile: () => hashObject(loaded),
})
expect(hashResult.hit).toBe(true)
})

View File

@@ -3,12 +3,14 @@ export { existsNonEmptyWantedLockfile } from './existsWantedLockfile.js'
export { getLockfileImporterId } from './getLockfileImporterId.js'
export { cleanGitBranchLockfiles } from './gitBranchLockfile.js'
export { convertToLockfileFile, convertToLockfileObject } from './lockfileFormatConverters.js'
export { getWantedLockfileName } from './lockfileName.js'
export * from './read.js'
export {
isEmptyLockfile,
writeCurrentLockfile,
writeLockfileFile,
writeLockfiles,
type WriteLockfilesResult,
writeWantedLockfile,
} from './write.js'
export { extractMainDocument } from './yamlDocuments.js'

View File

@@ -8,7 +8,7 @@ import yaml from 'js-yaml'
import { isEmpty } from 'ramda'
import writeFileAtomic from 'write-file-atomic'
import { convertToLockfileFile } from './lockfileFormatConverters.js'
import { convertToLockfileFile, convertToLockfileObject } from './lockfileFormatConverters.js'
import { getWantedLockfileName } from './lockfileName.js'
import { lockfileLogger as logger } from './logger.js'
import { sortLockfileKeys } from './sortLockfileKeys.js'
@@ -26,26 +26,34 @@ export function lockfileYamlDump (obj: object): string {
return yaml.dump(obj, LOCKFILE_YAML_FORMAT)
}
/**
* Returns the canonical post-write lockfile — structurally identical
* to what `readWantedLockfile` would parse back. Lets callers like
* the verification cache hash the as-saved form without re-reading.
*/
export async function writeWantedLockfile (
pkgPath: string,
wantedLockfile: LockfileObject,
opts?: {
useGitBranchLockfile?: boolean
mergeGitBranchLockfiles?: boolean
/** Pre-resolved filename; skips the `getWantedLockfileName` (and
* its `getCurrentBranch`) call when supplied. */
lockfileName?: string
}
): Promise<void> {
const wantedLockfileName: string = await getWantedLockfileName(opts)
): Promise<LockfileObject> {
const wantedLockfileName: string = opts?.lockfileName ?? await getWantedLockfileName(opts)
return writeLockfile(wantedLockfileName, pkgPath, wantedLockfile)
}
export async function writeCurrentLockfile (
virtualStoreDir: string,
currentLockfile: LockfileObject
): Promise<void> {
): Promise<LockfileObject | undefined> {
// empty lockfile is not saved
if (isEmptyLockfile(currentLockfile)) {
await rimraf(path.join(virtualStoreDir, 'lock.yaml'))
return
return undefined
}
await fs.mkdir(virtualStoreDir, { recursive: true })
return writeLockfile('lock.yaml', virtualStoreDir, currentLockfile)
@@ -55,7 +63,7 @@ async function writeLockfile (
lockfileFilename: string,
pkgPath: string,
wantedLockfile: LockfileObject
): Promise<void> {
): Promise<LockfileObject> {
const lockfilePath = path.join(pkgPath, lockfileFilename)
const lockfileToStringify = convertToLockfileFile(wantedLockfile)
@@ -69,9 +77,27 @@ async function writeLockfile (
// in the OS page cache and streaming stops at the first separator.
const envDoc = await streamReadFirstYamlDocument(lockfilePath)
const envPrefix = envDoc != null ? `${YAML_DOCUMENT_START}${envDoc}${YAML_DOCUMENT_SEPARATOR}` : ''
return writeFileAtomic(lockfilePath, `${envPrefix}${yamlDoc}`)
await writeFileAtomic(lockfilePath, `${envPrefix}${yamlDoc}`)
} else {
await writeFileAtomic(lockfilePath, yamlDoc)
}
return writeFileAtomic(lockfilePath, yamlDoc)
// YAML drops undefined on serialize, so the in-memory LockfileFile
// can carry fields (like an unset settings.dedupePeers) that won't
// survive a round-trip; strip them to mirror what the next reader
// will parse back.
return convertToLockfileObject(stripUndefinedDeep(lockfileToStringify) as LockfileFile)
}
function stripUndefinedDeep<T> (value: T): T {
if (value === null || typeof value !== 'object') return value
if (Array.isArray(value)) return value.map(stripUndefinedDeep) as unknown as T
const out: Record<string, unknown> = {}
for (const [k, v] of Object.entries(value as Record<string, unknown>)) {
if (v === undefined) continue
out[k] = stripUndefinedDeep(v)
}
return out as T
}
export function writeLockfileFile (
@@ -91,6 +117,19 @@ export function isEmptyLockfile (lockfile: LockfileObject): boolean {
return Object.values(lockfile.importers).every((importer) => isEmpty(importer.specifiers ?? {}) && isEmpty(importer.dependencies ?? {}))
}
export interface WriteLockfilesResult {
/**
* The canonical "as-saved" wanted lockfile — the inverse converter
* applied to the same object that was serialized to YAML. Hashing
* this is equivalent to hashing the lockfile the next install will
* load from disk (modulo undefined values that YAML drops, which any
* sensible canonicalization-then-hash routine should strip).
*/
wantedLockfile: LockfileObject
/** Same as above for the current lockfile, or undefined when it was skipped because empty. */
currentLockfile: LockfileObject | undefined
}
export async function writeLockfiles (
opts: {
wantedLockfile: LockfileObject
@@ -99,9 +138,11 @@ export async function writeLockfiles (
currentLockfileDir: string
useGitBranchLockfile?: boolean
mergeGitBranchLockfiles?: boolean
/** See {@link writeWantedLockfile}'s `lockfileName` option. */
wantedLockfileName?: string
}
): Promise<void> {
const wantedLockfileName: string = await getWantedLockfileName(opts)
): Promise<WriteLockfilesResult> {
const wantedLockfileName: string = opts.wantedLockfileName ?? await getWantedLockfileName(opts)
const wantedLockfilePath = path.join(opts.wantedLockfileDir, wantedLockfileName)
const currentLockfilePath = path.join(opts.currentLockfileDir, 'lock.yaml')
@@ -134,7 +175,12 @@ export async function writeLockfiles (
}
})(),
])
return
// Both files share the same source object; strip once and reuse.
const normalized = convertToLockfileObject(stripUndefinedDeep(wantedLockfileToStringify) as LockfileFile)
return {
wantedLockfile: normalized,
currentLockfile: isEmptyLockfile(opts.wantedLockfile) ? undefined : normalized,
}
}
logger.debug({
@@ -145,10 +191,13 @@ export async function writeLockfiles (
const currentLockfileToStringify = convertToLockfileFile(opts.currentLockfile)
const currentYamlDoc = yamlStringify(currentLockfileToStringify)
// Filtered-current callers (deps-restorer) can pass an empty
// current against a non-empty wanted; key off the current.
const currentIsEmpty = isEmptyLockfile(opts.currentLockfile)
await Promise.all([
writeFileAtomic(wantedLockfilePath, wantedYamlDoc),
(async () => {
if (isEmptyLockfile(opts.wantedLockfile)) {
if (currentIsEmpty) {
await rimraf(currentLockfilePath)
} else {
await fs.mkdir(path.dirname(currentLockfilePath), { recursive: true })
@@ -156,4 +205,10 @@ export async function writeLockfiles (
}
})(),
])
return {
wantedLockfile: convertToLockfileObject(stripUndefinedDeep(wantedLockfileToStringify) as LockfileFile),
currentLockfile: currentIsEmpty
? undefined
: convertToLockfileObject(stripUndefinedDeep(currentLockfileToStringify) as LockfileFile),
}
}

View File

@@ -98,6 +98,76 @@ test('writeLockfiles() when no specifiers but dependencies present', async () =>
expect(await readWantedLockfile(projectPath, { ignoreIncompatible: false })).toEqual(wantedLockfile)
})
test('writeWantedLockfile() returns the canonical lockfile — matches what readWantedLockfile produces, even when the input carries undefined optional fields', async () => {
// Cache-key contract: callers (today, the verification cache) need a
// hash of the *as-saved* lockfile, not the in-memory write object.
// Those two diverge specifically because YAML drops `undefined` on
// serialize. To exercise that drop, the fixture has to actually
// carry an explicit `undefined` — `settings.dedupePeers` here, the
// same field install-time code produces (see
// installing/deps-installer/src/install/index.ts where it's set to
// `opts.dedupePeers || undefined`). Without this, the test would
// happily pass against a writer that returned a near-canonical-but-
// still-divergent object.
const projectPath = temporaryDirectory()
const wantedLockfile = {
importers: {
'.': {
specifiers: { 'is-positive': '^1.0.0' },
dependencies: { 'is-positive': '1.0.0' },
},
},
lockfileVersion: LOCKFILE_VERSION,
settings: {
autoInstallPeers: true,
excludeLinksFromLockfile: false,
dedupePeers: undefined,
},
packages: {
'/is-positive@1.0.0': {
resolution: {
integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=',
},
},
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any
const written = await writeWantedLockfile(projectPath, wantedLockfile)
const loaded = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
expect(written).toEqual(loaded)
// Verify the canonicalization actually dropped the undefined field —
// toEqual is lenient about undefined-vs-missing, so check explicitly.
expect('dedupePeers' in (written.settings ?? {})).toBe(false)
})
test('writeLockfiles() return matches readWantedLockfile/readCurrentLockfile output', async () => {
const projectPath = temporaryDirectory()
const wantedLockfile = {
importers: {
'.': {
specifiers: { 'is-positive': '^1.0.0' },
dependencies: { 'is-positive': '1.0.0' },
},
},
lockfileVersion: LOCKFILE_VERSION,
packages: {
'/is-positive@1.0.0': {
resolution: { integrity: 'sha1-ChbBDewTLAqLCzb793Fo5VDvg/g=' },
},
},
}
const written = await writeLockfiles({
currentLockfile: wantedLockfile,
currentLockfileDir: projectPath,
wantedLockfile,
wantedLockfileDir: projectPath,
})
const loadedWanted = await readWantedLockfile(projectPath, { ignoreIncompatible: false })
const loadedCurrent = await readCurrentLockfile(projectPath, { ignoreIncompatible: false })
expect(written.wantedLockfile).toEqual(loadedWanted)
expect(written.currentLockfile).toEqual(loadedCurrent)
})
test('write does not use yaml anchors/aliases', async () => {
const projectPath = temporaryDirectory()
const wantedLockfile = {

View File

@@ -285,6 +285,18 @@ Run `just ready` (full suite) before handing the PR off.
re-exports such as `pub use submodule::*;` in a `lib.rs`. See the
"No star imports" section in `CODE_STYLE_GUIDE.md`.
### Comments
Same baseline as [`../AGENTS.md`](../AGENTS.md#comments): write code that explains itself; comments are for the non-obvious *why*, not a translation of the *what*.
Rust-specific defaults:
- **Doc comments (`///`, `//!`) document the contract.** Preconditions, postconditions, panics, the reason the function exists. They are not a re-narration of the body.
- **Do not restate at call sites what the callee's doc comment already says.** If `///` on the function says "no-op when …", the caller should not repeat that. Update the doc once; let every call site benefit.
- **`// SAFETY:`, `// TODO:`, and similar prefixes are the exception.** They signal hidden invariants or known follow-ups that a reader cannot recover from the code alone.
Prefer renaming, restructuring, or extracting a helper over leaving a comment. Reach for prose only when the right names and types genuinely cannot carry the information.
### Preserve existing method chains
When editing existing code, do not break a method chain (including `pipe-trait`

View File

@@ -120,6 +120,48 @@ describe('lockfile minimumReleaseAge verification', () => {
)
})
test('a fresh install records the just-written lockfile in the verification cache', async () => {
// Reproduces the "install foo, rm -rf node_modules, install" flow:
// the lockfile written by the first install must be recorded under
// its post-resolution content, otherwise the second install re-runs
// the registry round-trip even though the resolver already enforced
// the policy when picking those versions.
prepare({})
const cacheDir = path.resolve('pnpm-cache')
writeYamlFileSync('pnpm-workspace.yaml', {
minimumReleaseAge: 1,
minimumReleaseAgeStrict: true,
cacheDir,
})
execPnpmSync(
[PUBLIC_REGISTRY, 'add', 'is-positive@1.0.0'],
{ ...omitMinReleaseAgeEnv, expectSuccess: true }
)
const cacheFile = path.join(cacheDir, 'lockfile-verified.jsonl')
expect(fs.existsSync(cacheFile)).toBe(true)
const lines = fs.readFileSync(cacheFile, 'utf8').split('\n').filter(Boolean)
const records = lines.map((line) => JSON.parse(line) as {
lockfile: { hash: string, path: string }
policy: Record<string, unknown>
})
const lockfilePath = path.resolve('pnpm-lock.yaml')
const recordForLockfile = records.find((r) => r.lockfile.path === lockfilePath)
expect(recordForLockfile).toBeDefined()
expect(recordForLockfile!.policy).toMatchObject({ minimumReleaseAge: 1 })
// Re-running install completes without hitting the registry to
// re-verify is-positive. We can't directly observe the network skip,
// but a clean run with --offline confirms the cache short-circuit
// works end-to-end (the verifier would otherwise need a registry
// round-trip to evaluate the cutoff).
execPnpmSync(
[PUBLIC_REGISTRY, '--offline', 'install', '--frozen-lockfile'],
{ ...omitMinReleaseAgeEnv, expectSuccess: true }
)
})
test('loose mode rejects immature lockfile entries that are not on minimumReleaseAgeExclude', () => {
// The verifier now runs in loose mode too, so a lockfile produced under
// no policy that still has immature pins is rejected the same way

View File

@@ -246,7 +246,7 @@ async function deployFromSharedLockfile (
allowBuilds: opts.allowBuilds,
})
const filesToWrite: Array<Promise<void>> = [
const filesToWrite: Array<Promise<unknown>> = [
fs.promises.writeFile(
path.join(deployDir, 'package.json'),
JSON.stringify(deployFiles.manifest, undefined, 2) + '\n'