Consolidate pnpm lockfile reading

src/lib/lockfile/helpers/generate-pnpm-lockfile.test.ts

@@ -538,6 +538,24 @@ describe("generatePnpmLockfile", () => {
     ).rejects.toThrow();
   });
 
+  it("propagates lockfile read failures", async () => {
+    const readError = new Error("invalid lockfile");
+    readWantedLockfile_v9.mockRejectedValue(readError);
+
+    await expect(
+      generatePnpmLockfile({
+        workspaceRootDir: "/workspace",
+        targetPackageDir: "/workspace/apps/my-app",
+        isolateDir: "/workspace/apps/my-app/isolate",
+        internalDepPackageNames: [],
+        packagesRegistry: {},
+        targetPackageManifest: { name: "my-app", version: "1.0.0" },
+        majorVersion: 9,
+        includeDevDependencies: false,
+      }),
+    ).rejects.toBe(readError);
+  });
+
   it("should use Rush lockfile path when in a Rush workspace", async () => {
     isRushWorkspace.mockReturnValue(true);
     const lockfile = createMockLockfile();

src/lib/lockfile/helpers/generate-pnpm-lockfile.ts

@@ -3,20 +3,12 @@ import fs from "node:fs/promises";
 import path from "node:path";
 import {
   getLockfileImporterId as getLockfileImporterId_v8,
-  readWantedLockfile as readWantedLockfile_v8,
   writeWantedLockfile as writeWantedLockfile_v8,
 } from "pnpm_lockfile_file_v8";
-/**
- * The `_v9` alias names the lockfile format, not the package. It resolves to
- * `@pnpm/lockfile.fs`, the maintained successor of `@pnpm/lockfile-file`, whose
- * last release predates the two-document lockfile that pnpm 12 writes (see
- * issue #205). Both read and write lockfile format v9.
- */
 import type { EnvLockfile } from "pnpm_lockfile_file_v9";
 import {
   getLockfileImporterId as getLockfileImporterId_v9,
   readEnvLockfile as readEnvLockfile_v9,
-  readWantedLockfile as readWantedLockfile_v9,
   writeEnvLockfile as writeEnvLockfile_v9,
   writeWantedLockfile as writeWantedLockfile_v9,
 } from "pnpm_lockfile_file_v9";
@@ -34,6 +26,7 @@ import {
   getPnpmPatchedDependenciesOutput,
   usesPnpmWorkspacePatchedDependencies,
 } from "#/lib/patches/pnpm-patched-dependencies";
+import { readPnpmLockfile } from "../read-pnpm-lockfile";
 import { pnpmMapImporter } from "./pnpm-map-importer";
 
 /**
@@ -85,15 +78,9 @@ export async function generatePnpmLockfile({
 
     const lockfileRootDir = getPnpmLockfileDir(workspaceRootDir);
 
-    const lockfile = useVersion9
-      ? await readWantedLockfile_v9(lockfileRootDir, {
-          ignoreIncompatible: false,
-        })
-      : await readWantedLockfile_v8(lockfileRootDir, {
-          ignoreIncompatible: false,
-        });
+    const lockfile = await readPnpmLockfile(lockfileRootDir, majorVersion);
 
-    assert(lockfile, `No input lockfile found at ${workspaceRootDir}`);
+    assert(lockfile, `No input lockfile found at ${lockfileRootDir}`);
 
     const targetImporterId = useVersion9
       ? getLockfileImporterId_v9(workspaceRootDir, targetPackageDir)
@@ -308,9 +295,9 @@ async function copyEnvLockfile({
    * The reader resolves to null for a single-document lockfile — the ordinary
    * pnpm 9 to 11 case — but it throws rather than returning null when the file
    * exists and cannot be read or parsed: it special-cases only ENOENT, and the
-   * env document is passed through `yaml.load`. `readWantedLockfile` above has
-   * already succeeded by this point, so failing the whole isolate on the env
-   * document alone would turn a readable workspace into a hard error.
+   * env document is passed through `yaml.load`. The workspace lockfile read
+   * has already succeeded by this point, so failing the whole isolate on the
+   * env document alone would turn a readable workspace into a hard error.
    */
   let envLockfile: EnvLockfile | null;
 

src/lib/lockfile/read-pnpm-lockfile.test.ts

@@ -0,0 +1,97 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { readPnpmLockfile } from "./read-pnpm-lockfile";
+
+vi.mock("pnpm_lockfile_file_v8", () => ({
+  readWantedLockfile: vi.fn(),
+}));
+
+vi.mock("pnpm_lockfile_file_v9", () => ({
+  readWantedLockfile: vi.fn(),
+}));
+
+const { readWantedLockfile: readWantedLockfile_v8 } = vi.mocked(
+  await import("pnpm_lockfile_file_v8"),
+);
+const { readWantedLockfile: readWantedLockfile_v9 } = vi.mocked(
+  await import("pnpm_lockfile_file_v9"),
+);
+
+describe("readPnpmLockfile", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("returns the pnpm 8 lockfile for versions before pnpm 9", async () => {
+    const lockfile = {
+      lockfileVersion: 6,
+      importers: {},
+    };
+    readWantedLockfile_v8.mockResolvedValue(lockfile);
+
+    const result = await readPnpmLockfile("/workspace", 8);
+
+    expect(result).toBe(lockfile);
+    expect(readWantedLockfile_v8).toHaveBeenCalledOnce();
+    expect(readWantedLockfile_v8).toHaveBeenCalledWith("/workspace", {
+      ignoreIncompatible: false,
+    });
+    expect(readWantedLockfile_v9).not.toHaveBeenCalled();
+  });
+
+  it("returns the pnpm 9 lockfile for pnpm 9 and later", async () => {
+    const lockfile = {
+      lockfileVersion: "9.0",
+      importers: {},
+    };
+    readWantedLockfile_v9.mockResolvedValue(lockfile);
+
+    const result = await readPnpmLockfile("/workspace", 12);
+
+    expect(result).toBe(lockfile);
+    expect(readWantedLockfile_v9).toHaveBeenCalledOnce();
+    expect(readWantedLockfile_v9).toHaveBeenCalledWith("/workspace", {
+      ignoreIncompatible: false,
+    });
+    expect(readWantedLockfile_v8).not.toHaveBeenCalled();
+  });
+
+  it.each([
+    { majorVersion: 8, reader: readWantedLockfile_v8 },
+    { majorVersion: 9, reader: readWantedLockfile_v9 },
+  ])(
+    "preserves the missing-lockfile result for pnpm $majorVersion",
+    async ({ majorVersion, reader }) => {
+      reader.mockResolvedValue(null);
+
+      const result = await readPnpmLockfile("/workspace", majorVersion);
+
+      expect(result).toBeNull();
+    },
+  );
+
+  it.each([
+    { majorVersion: 8, reader: readWantedLockfile_v8 },
+    { majorVersion: 9, reader: readWantedLockfile_v9 },
+  ])(
+    "propagates pnpm $majorVersion lockfile read failures by default",
+    async ({ majorVersion, reader }) => {
+      const readError = new Error("invalid lockfile");
+      reader.mockRejectedValue(readError);
+
+      await expect(readPnpmLockfile("/workspace", majorVersion)).rejects.toBe(
+        readError,
+      );
+    },
+  );
+
+  it("returns the read error when the caller needs to report it", async () => {
+    const readError = new Error("invalid lockfile");
+    readWantedLockfile_v9.mockRejectedValue(readError);
+
+    const result = await readPnpmLockfile("/workspace", 11, {
+      onFailure: "return-error",
+    });
+
+    expect(result).toEqual({ readError });
+  });
+});

src/lib/lockfile/read-pnpm-lockfile.ts

@@ -0,0 +1,58 @@
+import { readWantedLockfile as readWantedLockfile_v8 } from "pnpm_lockfile_file_v8";
+/**
+ * The `_v9` alias names the lockfile format, not the package. It resolves to
+ * `@pnpm/lockfile.fs`, the maintained successor of `@pnpm/lockfile-file`.
+ * The older package cannot read the two-document lockfile that pnpm 12 writes.
+ */
+import { readWantedLockfile as readWantedLockfile_v9 } from "pnpm_lockfile_file_v9";
+
+type PnpmLockfile =
+  | Awaited<ReturnType<typeof readWantedLockfile_v8>>
+  | Awaited<ReturnType<typeof readWantedLockfile_v9>>;
+
+type ReadPnpmLockfileOptions =
+  | { onFailure?: "throw" }
+  | { onFailure: "return-error" };
+
+export function readPnpmLockfile(
+  lockfileDirectory: string,
+  majorVersion: number,
+  options?: { onFailure?: "throw" },
+): Promise<PnpmLockfile>;
+
+export function readPnpmLockfile(
+  lockfileDirectory: string,
+  majorVersion: number,
+  options: { onFailure: "return-error" },
+): Promise<PnpmLockfile | { readError: unknown }>;
+
+/**
+ * Read a workspace lockfile with the reader for its pnpm major version.
+ * A missing lockfile always resolves to `null`. Filesystem and parse errors
+ * throw by default or resolve to `{ readError }` with `return-error`. This
+ * keeps each consumer's failure policy explicit without exposing either
+ * versioned `readWantedLockfile` function.
+ */
+export async function readPnpmLockfile(
+  lockfileDirectory: string,
+  majorVersion: number,
+  options: ReadPnpmLockfileOptions = {},
+) {
+  try {
+    if (majorVersion >= 9) {
+      return await readWantedLockfile_v9(lockfileDirectory, {
+        ignoreIncompatible: false,
+      });
+    }
+
+    return await readWantedLockfile_v8(lockfileDirectory, {
+      ignoreIncompatible: false,
+    });
+  } catch (error) {
+    if (options.onFailure === "return-error") {
+      return { readError: error };
+    }
+
+    throw error;
+  }
+}

src/lib/patches/collect-installed-names-pnpm.test.ts

@@ -1,6 +1,12 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { collectInstalledNamesFromPnpmLockfile } from "./collect-installed-names-pnpm";
 
+const debugMock = vi.hoisted(() => vi.fn());
+
+vi.mock("#/lib/logger", () => ({
+  useLogger: vi.fn(() => ({ debug: debugMock })),
+}));
+
 vi.mock("pnpm_lockfile_file_v8", () => ({
   readWantedLockfile: vi.fn(() => Promise.resolve(null)),
   getLockfileImporterId: vi.fn(
@@ -16,6 +22,9 @@ vi.mock("pnpm_lockfile_file_v9", () => ({
 }));
 
 vi.mock("#/lib/utils", () => ({
+  getErrorMessage: vi.fn((error: unknown) =>
+    error instanceof Error ? error.message : String(error),
+  ),
   getPackageName: vi.fn((spec: string) => {
     if (spec.startsWith("@")) {
       const parts = spec.split("@");
@@ -286,6 +295,9 @@ describe("collectInstalledNamesFromPnpmLockfile", () => {
     });
 
     expect(result).toEqual(new Set());
+    expect(debugMock).toHaveBeenCalledWith(
+      "Failed to read pnpm lockfile for installed names: boom",
+    );
   });
 
   it("normalizes the target importer id before the isTarget check (Windows)", async () => {

src/lib/patches/collect-installed-names-pnpm.ts

@@ -1,15 +1,11 @@
 import path from "node:path";
-import {
-  getLockfileImporterId as getLockfileImporterId_v8,
-  readWantedLockfile as readWantedLockfile_v8,
-} from "pnpm_lockfile_file_v8";
-import {
-  getLockfileImporterId as getLockfileImporterId_v9,
-  readWantedLockfile as readWantedLockfile_v9,
-} from "pnpm_lockfile_file_v9";
+import { getLockfileImporterId as getLockfileImporterId_v8 } from "pnpm_lockfile_file_v8";
+import { getLockfileImporterId as getLockfileImporterId_v9 } from "pnpm_lockfile_file_v9";
+import { readPnpmLockfile } from "#/lib/lockfile/read-pnpm-lockfile";
 import { useLogger } from "#/lib/logger";
 import type { PackagesRegistry } from "#/lib/types";
 import {
+  getErrorMessage,
   getPackageName,
   getPnpmLockfileDir,
   isRushWorkspace,
@@ -49,9 +45,18 @@ export async function collectInstalledNamesFromPnpmLockfile({
     const isRush = isRushWorkspace(workspaceRootDir);
     const lockfileDir = getPnpmLockfileDir(workspaceRootDir);
 
-    const lockfile = useVersion9
-      ? await readWantedLockfile_v9(lockfileDir, { ignoreIncompatible: false })
-      : await readWantedLockfile_v8(lockfileDir, { ignoreIncompatible: false });
+    const lockfileResult = await readPnpmLockfile(lockfileDir, majorVersion, {
+      onFailure: "return-error",
+    });
+
+    if (lockfileResult && "readError" in lockfileResult) {
+      log.debug(
+        `Failed to read pnpm lockfile for installed names: ${getErrorMessage(lockfileResult.readError)}`,
+      );
+      return new Set();
+    }
+
+    const lockfile = lockfileResult;
 
     if (!lockfile) {
       log.debug("No pnpm lockfile available for installed-names walk");

src/lib/patches/copy-patches.ts

@@ -1,7 +1,6 @@
 import fs from "fs-extra";
 import path from "node:path";
-import { readWantedLockfile as readWantedLockfile_v8 } from "pnpm_lockfile_file_v8";
-import { readWantedLockfile as readWantedLockfile_v9 } from "pnpm_lockfile_file_v9";
+import { readPnpmLockfile } from "#/lib/lockfile/read-pnpm-lockfile";
 import { useLogger } from "#/lib/logger";
 import { usePackageManager } from "#/lib/package-manager";
 import { collectReachablePackageNames } from "#/lib/registry";
@@ -249,17 +248,16 @@ async function readLockfilePatchedDependencies(
   patchedDependencies?: Record<string, PatchFile | string>;
   readError?: unknown;
 }> {
-  try {
-    const { majorVersion } = usePackageManager();
-    const useVersion9 = majorVersion >= 9;
-    const lockfileDir = getPnpmLockfileDir(workspaceRootDir);
+  const { majorVersion } = usePackageManager();
+  const lockfileDir = getPnpmLockfileDir(workspaceRootDir);
 
-    const lockfile = useVersion9
-      ? await readWantedLockfile_v9(lockfileDir, { ignoreIncompatible: false })
-      : await readWantedLockfile_v8(lockfileDir, { ignoreIncompatible: false });
+  const result = await readPnpmLockfile(lockfileDir, majorVersion, {
+    onFailure: "return-error",
+  });
 
-    return { patchedDependencies: lockfile?.patchedDependencies };
-  } catch (error) {
-    return { readError: error };
+  if (result && "readError" in result) {
+    return result;
   }
+
+  return { patchedDependencies: result?.patchedDependencies };
 }