fix: stabilize Windows pacquet install tests (#12410)

- Replace lockfile env-document stream scanning with a FileHandle read loop that closes deterministically, including split-BOM handling.
- Align pacquet's default `virtualStoreDirMaxLength` with pnpm's Windows default.
- Forward pnpm's effective virtual store max length to delegated pacquet installs through `PNPM_CONFIG_VIRTUAL_STORE_DIR_MAX_LENGTH`, so currently published pacquet versions do not write mismatched `.modules.yaml` on Windows.

.changeset/slow-windows-lockfile-streams.md

@@ -1,6 +1,7 @@
 ---
 "@pnpm/lockfile.fs": patch
+"@pnpm/installing.commands": patch
 "pnpm": patch
 ---
 
-Wait for early-exited lockfile read streams to close before rewriting lockfiles.
+Close lockfile reads deterministically before rewriting lockfiles and keep pacquet's virtual store directory length aligned with pnpm on Windows.

installing/commands/src/installDeps.ts

@@ -119,6 +119,7 @@ export type InstallDepsOptions = Pick<Config,
 | 'configDependencies'
 | 'packageExtensions'
 | 'updateConfig'
+| 'virtualStoreDirMaxLength'
 > & Pick<ConfigContext,
 | 'allProjects'
 | 'allProjectsGraph'
@@ -245,6 +246,7 @@ export async function installDeps (
       packageName: pacquetConfigDepName,
       argv: { original: opts.argv.original, remain: opts.argv.remain ?? [] },
       isInstallCommand: opts.isInstallCommand === true,
+      virtualStoreDirMaxLength: opts.virtualStoreDirMaxLength,
     })
     : undefined
   const includeDirect = opts.includeDirect ?? {

installing/commands/src/runPacquet.ts

@@ -18,6 +18,13 @@ const streamParserWritable = streamParser as unknown as Writable
 
 export interface MakeRunPacquetOpts {
   lockfileDir: string
+  /**
+   * Effective pnpm config value. Forwarded through `PNPM_CONFIG_*` so
+   * pacquet writes the same `.modules.yaml` and virtual-store paths as
+   * the pnpm process that delegated to it, including Windows' shorter
+   * default.
+   */
+  virtualStoreDirMaxLength: number
   /**
    * Which `configDependencies` entry installed pacquet: either the
    * original unscoped `pacquet` or the official scoped
@@ -121,10 +128,7 @@ function makeRun (opts: MakeRunPacquetOpts): (callOpts?: RunPacquetCallOpts) =>
     // surface closely enough on that command that they're safe to pass
     // along. From `add`/`update`/`dedupe` we don't forward anything: those
     // commands carry flags pacquet's `install` doesn't recognize
-    // (`--save-dev`, `--save-peer`, etc.) which clap would reject. Either
-    // way pacquet picks up the settings users care about from
-    // `pnpm-workspace.yaml` / `.npmrc` on its own, so a non-install
-    // delegation isn't broken by the omission.
+    // (`--save-dev`, `--save-peer`, etc.) which clap would reject.
     const forwardedFlags = opts.isInstallCommand ? collectForwardedFlags(opts.argv) : []
     // In resolve mode pacquet does the resolution itself, so it must not
     // be pinned to the existing lockfile — drop both injected flags.
@@ -160,6 +164,7 @@ function makeRun (opts: MakeRunPacquetOpts): (callOpts?: RunPacquetCallOpts) =>
     logger.info({ message: banner, prefix: opts.lockfileDir })
     const child = spawn(pacquetBin, args, {
       cwd: opts.lockfileDir,
+      env: makePacquetEnv(opts),
       stdio: ['ignore', 'inherit', 'pipe'],
     })
     const filterResolved = callOpts?.filterResolvedProgress === true
@@ -197,6 +202,17 @@ function makeRun (opts: MakeRunPacquetOpts): (callOpts?: RunPacquetCallOpts) =>
   }
 }
 
+function makePacquetEnv (opts: MakeRunPacquetOpts): NodeJS.ProcessEnv {
+  const env = { ...process.env }
+  for (const key of Object.keys(env)) {
+    if (key.toLowerCase() === 'pnpm_config_virtual_store_dir_max_length') {
+      delete env[key]
+    }
+  }
+  env.PNPM_CONFIG_VIRTUAL_STORE_DIR_MAX_LENGTH = String(opts.virtualStoreDirMaxLength)
+  return env
+}
+
 /**
  * Path of the platform-specific native pacquet binary for the host. The
  * pacquet npm package ships a Node wrapper at `bin/pacquet` that uses

installing/commands/test/runPacquet.ts

@@ -23,6 +23,7 @@ function makeEngine (lockfileDir: string, packageName: 'pacquet' | '@pnpm/pacque
     packageName,
     argv: { original: [], remain: [] },
     isInstallCommand: true,
+    virtualStoreDirMaxLength: process.platform === 'win32' ? 60 : 120,
   })
 }
 

lockfile/fs/src/yamlDocuments.ts

@@ -1,4 +1,5 @@
-import { createReadStream, type ReadStream } from 'node:fs'
+import { type FileHandle, open } from 'node:fs/promises'
+import { StringDecoder } from 'node:string_decoder'
 import util from 'node:util'
 
 import stripBom from 'strip-bom'
@@ -6,69 +7,70 @@ import stripBom from 'strip-bom'
 export const YAML_DOCUMENT_SEPARATOR = '\n---\n'
 export const YAML_DOCUMENT_START = '---\n'
 
+const READ_BUFFER_SIZE = 64 * 1024
+
 /**
  * Reads the first YAML document from a multi-document YAML file using streaming.
  * The file must start with "---\n" to indicate it contains an env lockfile document.
  * Stops reading as soon as the second document separator is found.
  * Returns null if the file doesn't exist or doesn't start with "---\n".
  */
-export async function streamReadFirstYamlDocument (filePath: string): Promise<string | null> {
-  const stream = createReadStream(filePath, { encoding: 'utf8' })
-  const chunks = stream[Symbol.asyncIterator]()
+export async function streamReadFirstYamlDocument (filePath: string, readBufferSize = READ_BUFFER_SIZE): Promise<string | null> {
+  let fileHandle: FileHandle | undefined
   let buffer = ''
+  let firstChunk = true
   try {
-    // Phase 1: verify the file starts with "---\n"
-
-    for (let chunk = await chunks.next(); !chunk.done; chunk = await chunks.next()) { // eslint-disable-line no-await-in-loop
-      if (buffer.length === 0) {
-        // Strip BOM from the first chunk. Safe because the stream uses utf8 encoding,
-        // so the 3-byte BOM is decoded into a single \uFEFF character in the first chunk.
-        buffer = stripBom(chunk.value as string)
-      } else {
-        buffer += chunk.value
+    fileHandle = await open(filePath, 'r')
+    const decoder = new StringDecoder('utf8')
+    const readBuffer = Buffer.allocUnsafe(normalizeReadBufferSize(readBufferSize))
+    let position = 0
+    while (true) {
+      const { bytesRead } = await fileHandle.read(readBuffer, 0, readBuffer.length, position) // eslint-disable-line no-await-in-loop
+      if (bytesRead === 0) break
+      position += bytesRead
+      let chunk = decoder.write(readBuffer.subarray(0, bytesRead))
+      if (firstChunk && chunk.length > 0) {
+        // Strip BOM from the first chunk. Safe because the decoder uses utf8,
+        // so the 3-byte BOM is decoded into a single \uFEFF character.
+        chunk = stripBom(chunk)
+        firstChunk = false
       }
+      buffer += chunk
       // Normalize CRLF (Windows) to LF so document separator detection works.
       buffer = buffer.replace(/\r\n/g, '\n')
-      if (buffer.length >= YAML_DOCUMENT_START.length) break
-    }
-    if (!buffer.startsWith(YAML_DOCUMENT_START)) {
-      await closeStream(stream)
-      return null
-    }
-    // Phase 2: find the second "---" separator
-    let firstDocument: string | undefined
-    while (true) {
+      if (canRejectDocumentStart(buffer)) {
+        return null
+      }
       const sep = buffer.indexOf(YAML_DOCUMENT_SEPARATOR, YAML_DOCUMENT_START.length)
       if (sep !== -1) {
-        firstDocument = buffer.slice(YAML_DOCUMENT_START.length, sep)
-        break
+        return buffer.slice(YAML_DOCUMENT_START.length, sep)
       }
-      const chunk = await chunks.next() // eslint-disable-line no-await-in-loop
-      if (chunk.done) break
-      // Normalize CRLF (Windows) to LF so the separator search matches on Windows-checked-out files.
-      buffer = (buffer + chunk.value).replace(/\r\n/g, '\n')
     }
-    if (firstDocument != null) {
-      await closeStream(stream)
-      return firstDocument
+    const remainder = decoder.end()
+    if (remainder.length > 0) {
+      buffer += firstChunk ? stripBom(remainder) : remainder
+      buffer = buffer.replace(/\r\n/g, '\n')
     }
+    return null
   } catch (err: unknown) {
-    await closeStream(stream)
     if (util.types.isNativeError(err) && 'code' in err && err.code === 'ENOENT') {
       return null
     }
     throw err
+  } finally {
+    await fileHandle?.close().catch(() => {})
   }
-  await closeStream(stream)
-  return null
 }
 
-async function closeStream (stream: ReadStream): Promise<void> {
-  if (stream.closed) return
-  await new Promise<void>((resolve) => {
-    stream.once('close', resolve)
-    stream.destroy()
-  })
+function canRejectDocumentStart (buffer: string): boolean {
+  if (buffer.length < YAML_DOCUMENT_START.length) return false
+  if (buffer === '---\r') return false
+  return !buffer.startsWith(YAML_DOCUMENT_START)
+}
+
+function normalizeReadBufferSize (readBufferSize: number): number {
+  const size = Number.isFinite(readBufferSize) ? Math.floor(readBufferSize) : READ_BUFFER_SIZE
+  return size > 0 ? size : READ_BUFFER_SIZE
 }
 
 /**

lockfile/fs/test/yamlDocuments.test.ts

@@ -61,6 +61,22 @@ describe('streamReadFirstYamlDocument', () => {
     expect(result).toBe('foo: bar')
   })
 
+  test('handles BOM split across reads', async () => {
+    const dir = temporaryDirectory()
+    const filePath = path.join(dir, 'test.yaml')
+    fs.writeFileSync(filePath, '\uFEFF---\nfoo: bar\n---\nlockfileVersion: 9.0\n')
+    const result = await streamReadFirstYamlDocument(filePath, 2)
+    expect(result).toBe('foo: bar')
+  })
+
+  test.each([0, -1, Number.NaN])('falls back to default read buffer size for %p', async (readBufferSize) => {
+    const dir = temporaryDirectory()
+    const filePath = path.join(dir, 'test.yaml')
+    fs.writeFileSync(filePath, '---\nfoo: bar\n---\nlockfileVersion: 9.0\n')
+    const result = await streamReadFirstYamlDocument(filePath, readBufferSize)
+    expect(result).toBe('foo: bar')
+  })
+
   test('returns null for empty file', async () => {
     const dir = temporaryDirectory()
     const filePath = path.join(dir, 'test.yaml')
@@ -88,6 +104,14 @@ describe('streamReadFirstYamlDocument', () => {
     expect(result).toBe(envContent)
   })
 
+  test('handles CRLF document start split across reads', async () => {
+    const dir = temporaryDirectory()
+    const filePath = path.join(dir, 'test.yaml')
+    fs.writeFileSync(filePath, '---\r\nfoo: bar\r\n---\r\nlockfileVersion: 9.0\r\n')
+    const result = await streamReadFirstYamlDocument(filePath, 4)
+    expect(result).toBe('foo: bar')
+  })
+
   test('handles BOM with CRLF line endings', async () => {
     const dir = temporaryDirectory()
     const filePath = path.join(dir, 'test.yaml')

pacquet/crates/config/src/defaults.rs

@@ -217,17 +217,12 @@ pub fn default_modules_cache_max_age() -> u64 {
     10080
 }
 
-/// Default `virtualStoreDirMaxLength` matching pnpm's fallback at
-/// <https://github.com/pnpm/pnpm/blob/1819226b51/installing/modules-yaml/src/index.ts#L101-L103>.
-///
-/// Kept as a free function (not a re-export of
-/// `pacquet_modules_yaml::DEFAULT_VIRTUAL_STORE_DIR_MAX_LENGTH`) so
-/// `pacquet-config` doesn't pull in the modules-yaml crate just for one
-/// integer. Both copies must agree; the modules-yaml side carries the
-/// same upstream link.
+/// Default `virtualStoreDirMaxLength` matching pnpm's platform-aware
+/// config default at
+/// <https://github.com/pnpm/pnpm/blob/d50d691e5a/config/reader/src/index.ts#L216>.
 #[must_use]
 pub fn default_virtual_store_dir_max_length() -> u64 {
-    120
+    if cfg!(windows) { 60 } else { 120 }
 }
 
 /// Default `peersSuffixMaxLength` matching pnpm's fallback at

pacquet/crates/config/src/lib.rs

@@ -500,8 +500,8 @@ pub struct Config {
     /// flat name would exceed this many bytes, the tail is replaced
     /// with a 32-char sha256 hash so the path stays within filesystem
     /// limits (macOS / ext4 cap component names at 255 bytes; pnpm
-    /// defaults to 120 to leave headroom for `node_modules/<name>`
-    /// suffixes appended below).
+    /// defaults to 60 on Windows and 120 elsewhere to leave headroom
+    /// for `node_modules/<name>` suffixes appended below).
     ///
     /// Configurable via `virtualStoreDirMaxLength` in
     /// `pnpm-workspace.yaml`, global `config.yaml`, or
@@ -511,7 +511,7 @@ pub struct Config {
     /// The same value is persisted into `node_modules/.modules.yaml`
     /// so subsequent installs see the user's pick.
     ///
-    /// Default value is 120.
+    /// Default value is 60 on Windows and 120 otherwise.
     #[default(_code = "default_virtual_store_dir_max_length()")]
     pub virtual_store_dir_max_length: u64,
 

pacquet/crates/config/src/tests.rs

@@ -1504,16 +1504,14 @@ pub fn pnpm_config_hoist_false_clears_hoist_pattern() {
     );
 }
 
-/// `virtualStoreDirMaxLength` defaults to 120 — same value pnpm
-/// writes when nothing is configured. The constant lives in
-/// `pacquet-modules-yaml`; this asserts the config side carries
-/// the matching default so a fresh install produces the same
-/// virtual-store dirnames as pnpm.
+/// `virtualStoreDirMaxLength` defaults to the same platform-aware value
+/// pnpm uses when nothing is configured.
 #[test]
-pub fn virtual_store_dir_max_length_defaults_to_120() {
+pub fn virtual_store_dir_max_length_matches_pnpm_default() {
     let tmp = tempdir().unwrap();
     let config = Config::new().current::<HostNoHome>(tmp.path()).expect("loads");
-    assert_eq!(config.virtual_store_dir_max_length, 120);
+    let expected = if cfg!(windows) { 60 } else { 120 };
+    assert_eq!(config.virtual_store_dir_max_length, expected);
 }
 
 /// `virtualStoreDirMaxLength` in `pnpm-workspace.yaml` overrides

pacquet/crates/package-manager/src/install/tests.rs

@@ -888,7 +888,10 @@ async fn install_writes_modules_yaml() {
     // `modules_dir`, so a relative on-disk value round-trips back
     // to the absolute install-time path.
     assert_eq!(emitted_virtual_store_dir, virtual_store_dir.to_string_lossy());
-    assert_eq!(virtual_store_dir_max_length, DEFAULT_VIRTUAL_STORE_DIR_MAX_LENGTH);
+    assert_eq!(
+        virtual_store_dir_max_length,
+        pacquet_config::default_virtual_store_dir_max_length(),
+    );
     assert_eq!(
         registries.as_ref().and_then(|r| r.get("default")).map(String::as_str),
         Some(config.registry.as_str()),

pacquet/crates/package-manager/src/virtual_store_layout.rs

@@ -79,8 +79,9 @@ pub struct VirtualStoreLayout {
     /// the escaped filename exceeds this many bytes, the tail is
     /// replaced with a 32-char sha256 hash so the directory name fits
     /// within filesystem limits (macOS / ext4 cap component names at
-    /// 255 bytes, but pnpm defaults to 120 to leave headroom for the
-    /// `<name>@<version>/` suffix appended below).
+    /// 255 bytes, but pnpm defaults to 60 on Windows and 120 elsewhere
+    /// to leave headroom for the `<name>@<version>/` suffix appended
+    /// below).
     ///
     /// [`PkgNameVerPeer::to_virtual_store_name`]: pacquet_lockfile::PkgNameVerPeer::to_virtual_store_name
     virtual_store_dir_max_length: usize,